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

NAS-129794 / 24.10 / Use pre-defined constant for UID validator #13948

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

anodos325
Copy link
Contributor

We shouldn't have second definition of TRUENAS_IDMAP_DEFAULT_LOW.

We shouldn't have second definition of TRUENAS_IDMAP_DEFAULT_LOW.
@anodos325 anodos325 added the jira label Jun 28, 2024
@anodos325 anodos325 requested a review from a team June 28, 2024 15:23
@bugclerk
Copy link
Contributor

@bugclerk bugclerk changed the title Use pre-defined constant for UID validator NAS-129794 / 24.10 / Use pre-defined constant for UID validator Jun 28, 2024
Copy link
Contributor

@themylogin themylogin left a comment

Choose a reason for hiding this comment

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

Changing it there will retroactively change API definition for already released APIs, that should not be the case.

If it is not a part of API definition, let's validate it in the code, not in the API definitions,

@anodos325
Copy link
Contributor Author

Changing it there will retroactively change API definition for already released APIs, that should not be the case.

If it is not a part of API definition, let's validate it in the code, not in the API definitions,

Obviously if someone is changing constants they'll need to be aware of its impact on other areas of the product. I don't see any reason to legitimately allow different ID ranges based on which endpoint an API consumer chooses to use.

@themylogin
Copy link
Contributor

if someone is changing constants they'll need to be aware of its impact on other areas of the product.

We took an obligation to have backwards-compatible API. One of the methods we decided to use in order to comply with it is to never import anything in the API definition.

This value should probably be validated in the create/update method itself, not in the API schema.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants