New proposed dataset authorization matrix#2722
Merged
nitrosx merged 2 commits intoMay 6, 2026
Merged
Conversation
…orization_documentation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In the new environment variables section, consider tightening the wording (e.g., "present" → "presents", "users' groups that acquired" → "user groups that acquire") and fix minor grammar like "effects all the sub-systems" → "affects all the subsystems" to improve clarity.
- In the Legacy mapping list, it may be worth explicitly stating how conflicts are resolved when both legacy and new environment variables are set (e.g., which takes precedence) so operators understand the effective configuration.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the new environment variables section, consider tightening the wording (e.g., "present" → "presents", "users' groups that acquired" → "user groups that acquire") and fix minor grammar like "effects all the sub-systems" → "affects all the subsystems" to improve clarity.
- In the Legacy mapping list, it may be worth explicitly stating how conflicts are resolved when both legacy and new environment variables are set (e.g., which takes precedence) so operators understand the effective configuration.
## Individual Comments
### Comment 1
<location path="docs/developer-guide/authorization/authorization_datasets.md" line_range="147-149" />
<code_context>
- Delete
- Anonymous -> Authenticated -> Dataset Delete Basic -> Dataset Delete Privileged -> Delete
+
+## Environmental Variables
+
+The following list present the environmental variables that should be configured to setup the special groups listed in the previous sections.
</code_context>
<issue_to_address>
**suggestion (typo):** Consider using the more conventional term "Environment Variables" for the heading.
Both forms are correct, but using the standard term will better match typical technical documentation and user expectations.
```suggestion
## Environment Variables
The following list present the environment variables that should be configured to setup the special groups listed in the previous sections.
```
</issue_to_address>
### Comment 2
<location path="docs/developer-guide/authorization/authorization_datasets.md" line_range="149" />
<code_context>
+
+## Environmental Variables
+
+The following list present the environmental variables that should be configured to setup the special groups listed in the previous sections.
+Each variable is a comma separated list of the users' groups that acquired the special permissions linked to the special group.
+
</code_context>
<issue_to_address>
**issue (typo):** Fix minor grammatical issues in this sentence ("present" → "presents", "setup" → "set up").
Consider: "The following list presents the environment variables that should be configured to set up the special groups listed in the previous sections."
```suggestion
The following list presents the environment variables that should be configured to set up the special groups listed in the previous sections.
```
</issue_to_address>
### Comment 3
<location path="docs/developer-guide/authorization/authorization_datasets.md" line_range="150" />
<code_context>
+## Environmental Variables
+
+The following list present the environmental variables that should be configured to setup the special groups listed in the previous sections.
+Each variable is a comma separated list of the users' groups that acquired the special permissions linked to the special group.
+
+- DATASET_READ_PRIVILEGED_GROUPS: groups with __Dataset Read Privileged__ permissions
</code_context>
<issue_to_address>
**suggestion (typo):** Tighten wording and fix hyphenation and verb tense in this sentence.
For example: “Each variable is a comma-separated list of the user groups that acquire the special permissions linked to the special group.”
Suggested implementation:
```
## Environmental Variables
The following list presents the environmental variables that should be configured to set up the special groups listed in the previous sections.
Each variable is a comma-separated list of the user groups that acquire the special permissions linked to the special group.
```
```
- ADMIN_GROUPS: groups with __Admin__ permissions. This variable affects all the sub-systems.
- DELETE_GROUPS: groups with __Delete__ permissions. This variable affects all the sub-systems.
```
</issue_to_address>
### Comment 4
<location path="docs/developer-guide/authorization/authorization_datasets.md" line_range="160-161" />
<code_context>
+- DATASET_UPDATE_PRIVILEGED_GROUPS: groups with __Dataset Update Privileged__ permissions
+- DATASET_DELETE_BASIC_GROUPS: groups with __Dataset Delete Basic__ permissions
+- DATASET_DELETE_PRIVILEGED_GROUPS: groups with __Dataset Delete Privileged__ permissions
+- ADMIN_GROUPS: groups with __Admin__ permissions. This variable effects all the sub-systems.
+- DELETE_GROUPS: groups with __Delete__ permissions. This variable effects all the sub-systems.
+
+## Legacy
</code_context>
<issue_to_address>
**issue (typo):** Use "affects" instead of "effects" and consider simplifying "sub-systems" to "subsystems."
For example: `This variable affects all the subsystems.` Also, please apply the same wording to both ADMIN_GROUPS and DELETE_GROUPS descriptions for consistency.
```suggestion
- ADMIN_GROUPS: groups with __Admin__ permissions. This variable affects all the subsystems.
- DELETE_GROUPS: groups with __Delete__ permissions. This variable affects all the subsystems.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+147
to
+149
| ## Environmental Variables | ||
|
|
||
| The following list present the environmental variables that should be configured to setup the special groups listed in the previous sections. |
There was a problem hiding this comment.
suggestion (typo): Consider using the more conventional term "Environment Variables" for the heading.
Both forms are correct, but using the standard term will better match typical technical documentation and user expectations.
Suggested change
| ## Environmental Variables | |
| The following list present the environmental variables that should be configured to setup the special groups listed in the previous sections. | |
| ## Environment Variables | |
| The following list present the environment variables that should be configured to setup the special groups listed in the previous sections. |
|
|
||
| ## Environmental Variables | ||
|
|
||
| The following list present the environmental variables that should be configured to setup the special groups listed in the previous sections. |
There was a problem hiding this comment.
issue (typo): Fix minor grammatical issues in this sentence ("present" → "presents", "setup" → "set up").
Consider: "The following list presents the environment variables that should be configured to set up the special groups listed in the previous sections."
Suggested change
| The following list present the environmental variables that should be configured to setup the special groups listed in the previous sections. | |
| The following list presents the environment variables that should be configured to set up the special groups listed in the previous sections. |
| ## Environmental Variables | ||
|
|
||
| The following list present the environmental variables that should be configured to setup the special groups listed in the previous sections. | ||
| Each variable is a comma separated list of the users' groups that acquired the special permissions linked to the special group. |
There was a problem hiding this comment.
suggestion (typo): Tighten wording and fix hyphenation and verb tense in this sentence.
For example: “Each variable is a comma-separated list of the user groups that acquire the special permissions linked to the special group.”
Suggested implementation:
## Environmental Variables
The following list presents the environmental variables that should be configured to set up the special groups listed in the previous sections.
Each variable is a comma-separated list of the user groups that acquire the special permissions linked to the special group.
- ADMIN_GROUPS: groups with __Admin__ permissions. This variable affects all the sub-systems.
- DELETE_GROUPS: groups with __Delete__ permissions. This variable affects all the sub-systems.
Comment on lines
+160
to
+161
| - ADMIN_GROUPS: groups with __Admin__ permissions. This variable effects all the sub-systems. | ||
| - DELETE_GROUPS: groups with __Delete__ permissions. This variable effects all the sub-systems. |
There was a problem hiding this comment.
issue (typo): Use "affects" instead of "effects" and consider simplifying "sub-systems" to "subsystems."
For example: This variable affects all the subsystems. Also, please apply the same wording to both ADMIN_GROUPS and DELETE_GROUPS descriptions for consistency.
Suggested change
| - ADMIN_GROUPS: groups with __Admin__ permissions. This variable effects all the sub-systems. | |
| - DELETE_GROUPS: groups with __Delete__ permissions. This variable effects all the sub-systems. | |
| - ADMIN_GROUPS: groups with __Admin__ permissions. This variable affects all the subsystems. | |
| - DELETE_GROUPS: groups with __Delete__ permissions. This variable affects all the subsystems. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR includes an update to the new revised Dataset Authorization matrix.
Summary by Sourcery
Update dataset authorization documentation to reflect new endpoints and environment-based configuration for special permission groups.
Documentation: