Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Include feature flags for all plugins using src attributes #6304

Merged
merged 5 commits into from
Apr 13, 2023
Merged

Include feature flags for all plugins using src attributes #6304

merged 5 commits into from
Apr 13, 2023

Conversation

groszewn
Copy link
Contributor

@groszewn groszewn commented Apr 11, 2023 •
edited
Loading

Updates individual HTML elements to include client feature flags as query parameters on their src attributes.

Validated via a local tensorboard that the requests generated from these elements include the client feature flags as query parameters.

@groszewn groszewn force-pushed the client_feature_flags branch from 7694e64 to 62fe3ce Compare April 11, 2023 12:38
@groszewn groszewn marked this pull request as ready for review April 11, 2023 13:04
@groszewn groszewn requested a review from bmd3k April 11, 2023 13:04
bmd3k
bmd3k reviewed Apr 11, 2023
View reviewed changes
Copy link
Contributor

@bmd3k bmd3k left a comment

Choose a reason for hiding this comment

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

We discussed offline that there are more places where this query param will need to be set. We agreed to come up with some way to encapsulate the logic so it doesn't have to be repeated everywhere.

@groszewn groszewn changed the title Include feature flags for image and audio plugins on src attributes Include feature flags for image/audio/graph plugins on src attributes Apr 11, 2023
@groszewn groszewn force-pushed the client_feature_flags branch 2 times, most recently from 4e03aed to 47eba7d Compare April 11, 2023 14:46
@groszewn
Copy link
Contributor Author

@bmd3k as discussed offline, added a pluginRouteForSrc method to the router object which automatically includes client feature flags as query parameters along with specified search params. This requires minimal changes in the underlying plugins (i.e. just swapping pluginRoute -> pluginRouteForSrc).

@bmd3k bmd3k self-requested a review April 11, 2023 15:31
@groszewn groszewn changed the title Include feature flags for image/audio/graph plugins on src attributes Include feature flags for all plugins using src attributes Apr 11, 2023
@groszewn
Copy link
Contributor Author

@bmd3k I've updated this PR to include all instances of URLs being generated with getRouter().pluginRoute() that isn't followed by a call to requestManager under the plugins directory.

@bmd3k bmd3k self-requested a review April 13, 2023 11:11
@groszewn groszewn force-pushed the client_feature_flags branch from 8a00428 to 9969dec Compare April 13, 2023 11:19
@groszewn groszewn merged commit aa07984 into tensorflow:master Apr 13, 2023
dna2github pushed a commit to dna2fork/tensorboard that referenced this pull request May 1, 2023
@groszewn @dna2github
Include feature flags for all plugins using src attributes (tensorflo…
decdaef 
…w#6304)

Updates individual HTML elements to include client feature flags as
query parameters on their src attributes.

Validated via a local tensorboard that the requests generated from these
elements include the client feature flags as query parameters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bmd3k bmd3k bmd3k approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@groszewn @bmd3k