Skip to content

Refactor AuthContext creation into standalone AuthContextMiddleware. #6131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 5, 2023

Conversation

bmd3k
Copy link
Contributor

@bmd3k bmd3k commented Jan 5, 2023

The TensorBoard application layer creates a middleware that injects AuthContext into the RequestContext. It would be useful to expose this middleware for other usage - there are at least a couple test files in the internal code base that could make use of it.

This change refactors the middleware into its own file for reuse, naming it AuthContextMiddleware and giving it its own devoted test.

@bmd3k bmd3k requested a review from arcra January 5, 2023 15:46
def _create_auth_provider_verifier_app(expected_auth_key):
"""Generates a WSGI application for verifying AuthContextMiddleware.

It should be placed downstream from the AuthContextMiddleware. It will
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I wasn't sure of what this "downstream" terminology meant. For some reason I was imagining the opposite of what you're doing in the test below, although perhaps that doesn't make much sense.

This is just for this test/file, so it doesn't matter much, but consider whether a different phrasing or a 2-line example would be worth adding.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried to reword since adding an example just duplicates the code below. Since the WSGI PEP (https://peps.python.org/pep-0333/) makes a passing reference to the "Chain of Responsibility" pattern, I reworded in those terms.

@bmd3k bmd3k merged commit 4c929f6 into tensorflow:master Jan 5, 2023
yatbear pushed a commit to yatbear/tensorboard that referenced this pull request Mar 27, 2023
…ensorflow#6131)

The TensorBoard application layer creates a middleware that injects
AuthContext into the RequestContext. It would be useful to expose this
middleware for other usage - there are at least a couple test files in
the internal code base that could make use of it.

This change refactors the middleware into its own file for reuse, naming
it AuthContextMiddleware and giving it its own devoted test.
dna2github pushed a commit to dna2fork/tensorboard that referenced this pull request May 1, 2023
…ensorflow#6131)

The TensorBoard application layer creates a middleware that injects
AuthContext into the RequestContext. It would be useful to expose this
middleware for other usage - there are at least a couple test files in
the internal code base that could make use of it.

This change refactors the middleware into its own file for reuse, naming
it AuthContextMiddleware and giving it its own devoted test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants