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.

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

ui: Make sure we pass the nspace through to the API for nodes #9488

Merged
merged 2 commits into from
Jan 5, 2021

Conversation

johncowen
Copy link
Contributor

Manual backport of #9410 to 1.8.x

@johncowen johncowen added backport/1.7 theme/ui Anything related to the UI labels Jan 4, 2021
@johncowen johncowen marked this pull request as ready for review January 4, 2021 17:16
@johncowen johncowen requested a review from kaxcode January 4, 2021 17:16
@johncowen
Copy link
Contributor Author

@mikemorris , just checking in to make sure this is how you'd want to do this? I did this completely manually and copied over the same change log (with this PRs number)

Thinking if I label as backport to 1.7 from here it should go cleanly from 1.8 to 1.7

Copy link
Contributor

@kaxcode kaxcode left a comment

Choose a reason for hiding this comment

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

LGTM

@mikemorris
Copy link
Contributor

LGTM for backport targeting 1.8.x!

Thinking if I label as backport to 1.7 from here it should go cleanly from 1.8 to 1.7

I believe the backport label flow only triggers on PRs targeting master currently, @alvin-huang may have thoughts on whether we could/should enable auto-backports like 1.8.x->1.7.x in the future, but cherry-picking this onto 1.7.x after merging would be the path for now.

@johncowen
Copy link
Contributor Author

Thanks!

I believe the backport label flow only triggers on PRs targeting master

Ah cool I did wonder, I can just manually cherry pick if not 👍

@johncowen johncowen merged commit af335e7 into release/1.8.x Jan 5, 2021
@johncowen johncowen deleted the ui/bugfix/cp-9410 branch January 5, 2021 15:54
johncowen added a commit that referenced this pull request Jan 5, 2021
Nodes themselves are not namespaced, so we'd originally assumed we did not need to pass through the ns query parameter when listing or viewing nodes.

As it turns out the API endpoints we use to list and view nodes (and related things) return things that are namespaced, therefore any API requests for nodes do require a the ns query parameter to be passed through to the request.

This PR adds the necessary ns query param to all things Node, apart from the querying for the leader which only returns node related information.
johncowen added a commit that referenced this pull request Jan 6, 2021
…#9499)

Nodes themselves are not namespaced, so we'd originally assumed we did not need to pass through the ns query parameter when listing or viewing nodes.

As it turns out the API endpoints we use to list and view nodes (and related things) return things that are namespaced, therefore any API requests for nodes do require a the ns query parameter to be passed through to the request.

This PR adds the necessary ns query param to all things Node, apart from the querying for the leader which only returns node related information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants