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-131143 / 25.04 / Always use python serializer and preserve types #14542

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

themylogin
Copy link
Contributor

…htly different results

@bugclerk bugclerk changed the title Remove duplicate privileges_group_mapping method that produces slig… NAS-131143 / 25.04 / Remove duplicate privileges_group_mapping method that produces slig… Sep 19, 2024
@bugclerk
Copy link
Contributor

@anodos325
Copy link
Contributor

Won't this change the type for roles in user.query and group.query response?

@@ -1612,11 +1612,13 @@ async def group_extend(self, group, ctx):
group['name'] = group['group']
group['users'] = ctx['memberships'].get(group['id'], [])

privilege_mappings = privileges_group_mapping(ctx['privileges'], [group['gid']], 'local_groups')
privilege_mappings = await self.middleware.call(
Copy link
Contributor

@anodos325 anodos325 Sep 19, 2024

Choose a reason for hiding this comment

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

I'd rather not add an extra middleware call per-group here especially when the op is so trivial. We can make the util version create a set instead and then convert to list for the user / group query output so that we're not changing things in the API response.

@themylogin themylogin changed the title NAS-131143 / 25.04 / Remove duplicate privileges_group_mapping method that produces slig… NAS-131143 / 25.04 / Always use python serializer and preserve types Sep 19, 2024
Copy link
Contributor

@anodos325 anodos325 left a comment

Choose a reason for hiding this comment

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

Looks good.

@themylogin themylogin merged commit 51b14d6 into master Sep 20, 2024
3 checks passed
@themylogin themylogin deleted the NAS-131143 branch September 20, 2024 10:38
@bugclerk
Copy link
Contributor

JIRA ticket https://ixsystems.atlassian.net/browse/NAS-131143 is targeted to the following versions which have not received their corresponding PRs: 24.10.0

@bugclerk
Copy link
Contributor

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Sep 20, 2024
@themylogin
Copy link
Contributor Author

time 4:00

@bugclerk
Copy link
Contributor

Time tracking added.

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

Successfully merging this pull request may close these issues.

3 participants