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

Fixes #22403 - every API endpoint can set current context #5208

Merged
merged 1 commit into from
Feb 23, 2018

Conversation

ares
Copy link
Member

@ares ares commented Jan 24, 2018

@iNecas @mbacovsky this is what we discussed yesterday, this seems as most straightforward way to add this to every request definition. It also adds it to resources which are not taxable, e.g. architectures API, but that is consistent with context switcher on architectures page in UI.

@theforeman-bot
Copy link
Member

Issues: #22403

@ares
Copy link
Member Author

ares commented Jan 24, 2018

We could perhaps improve the texts to let users know that nil mean any context. I'm open to suggestions

Copy link
Member

@dLobatog dLobatog left a comment

Choose a reason for hiding this comment

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

I assume this is related to the mails where there's a discussion to remove the scoping by taxonomies by default on the API (all taxonomies selected by default) - should that be included here?

@ares
Copy link
Member Author

ares commented Jan 24, 2018

It's not really related, the API already works even without this definition, but it's not documented and hammer does not render these arguments. With this PR all hammer commands would have --organization-id and --location-ld to specify current context.

@ares
Copy link
Member Author

ares commented Feb 22, 2018

removing WoC, this is unrelated change as mentioned a month ago, could someone please take a look? the default scope for API was removed separately already

@iNecas
Copy link
Member

iNecas commented Feb 22, 2018

Ack from me. @dLobatog any other objections?

@dLobatog
Copy link
Member

@iNecas none, merging - thanks @ares

@dLobatog dLobatog merged commit 8bd79a1 into theforeman:develop Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants