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

[Serverless] Update role name description #2888

Conversation

slobodanadamovic
Copy link
Contributor

@slobodanadamovic slobodanadamovic commented Sep 12, 2024

I've update the role name parameter description to point out the restrictions we have. But since the specification is generated from SecurityPutRoleRequest.ts (and is shared), it makes this proposed change true for Serverless, but completely wrong for stateful.

Also this change is fragile, anytime someone runs make contrib it will override the specs for stateful as well.

It's not nice, but I could potentially word it in a way to mention that this restriction only applies in Serverless. LMKWYT?

Ideally, we should have this documented in the dedicated docs for Serverles.

Copy link
Contributor

@n1v0lg n1v0lg left a comment

Choose a reason for hiding this comment

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

LGTM -- just in case, can you run it my @pquentin before merging? (Only when we're ready to merge ofc)

Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! We don't have the option to have serverless-specific descriptions. Any reason not to add "On Elasticsearch Serverless, the role name must ..."?

@slobodanadamovic slobodanadamovic marked this pull request as ready for review October 3, 2024 08:29
@slobodanadamovic
Copy link
Contributor Author

Any reason not to add "On Elasticsearch Serverless, the role name must ..."?

I wasn't sure if there is a better way to document this and avoid mentioning Serverless. But I'm fine with this proposal.

@slobodanadamovic slobodanadamovic merged commit c25af2c into elastic:mark-role-apis-serverless-available Oct 3, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants