quiltdata / quilt

Quilt is a data mesh for connecting people with actionable data
https://quiltdata.com
Apache License 2.0
1.32k stars 91 forks source link

GraphQL-based quilt3.admin API #3990

Closed sir-sigurd closed 2 months ago

sir-sigurd commented 3 months ago

Description

TODO

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 88.25911% with 87 lines in your changes missing coverage. Please review.

Project coverage is 37.46%. Comparing base (64e081c) to head (e38e644). Report is 4 commits behind head on master.

Files Patch % Lines
...python/quilt3/admin/_graphql_client/base_client.py 26.74% 63 Missing :warning:
.../python/quilt3/admin/_graphql_client/exceptions.py 55.00% 18 Missing :warning:
.../python/quilt3/admin/_graphql_client/base_model.py 69.23% 4 Missing :warning:
api/python/quilt3/admin/users.py 98.03% 1 Missing :warning:
api/python/tests/test_admin_api.py 98.98% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #3990 +/- ## ========================================== + Coverage 36.68% 37.46% +0.77% ========================================== Files 724 746 +22 Lines 32254 32717 +463 Branches 4773 4604 -169 ========================================== + Hits 11833 12256 +423 - Misses 19259 19798 +539 + Partials 1162 663 -499 ``` | [Flag](https://app.codecov.io/gh/quiltdata/quilt/pull/3990/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quiltdata) | Coverage Δ | | |---|---|---| | [api-python](https://app.codecov.io/gh/quiltdata/quilt/pull/3990/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quiltdata) | `90.62% <88.25%> (-0.33%)` | :arrow_down: | | [catalog](https://app.codecov.io/gh/quiltdata/quilt/pull/3990/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quiltdata) | `10.71% <ø> (+<0.01%)` | :arrow_up: | | [lambda](https://app.codecov.io/gh/quiltdata/quilt/pull/3990/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quiltdata) | `88.18% <ø> (+0.21%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quiltdata#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

drernie commented 3 months ago

This looks great! I assume in the final version it will be something like "quilt3.admin"; 'adminql' is too much information

My main concern is that I find the name and type of "role" confusing; when do I use a role_id vs a role_name?

Is it possible to:

sir-sigurd commented 2 months ago

@nl0 do you think anything needs to be done other than tests?

nl0 commented 2 months ago

@nl0 do you think anything needs to be done other than tests?

idk, seems good enough. maybe add some more management commands? e.g. make and admin, edit an email, etc?

sir-sigurd commented 2 months ago

CI currently fail because of https://github.com/jd/tenacity/issues/471

sir-sigurd commented 2 months ago

@nl0 does it make sense to rename some functions set_role -> set_user_role add_roles -> add_user_roles remove_roles -> remove_user_roles ?

or maybe use prefixes for consistency? i.e.

get_user -> user_get get_users -> user_list set_role -> user_set_role ?

nl0 commented 2 months ago

does it make sense to rename some functions?

yeah i think so

or maybe use prefixes for consistency? i.e.

that feels like a good intent, but the resulting names don't look too nice, so... your call. imo it would look nice if we had some sort of a namespace (e.g. admin.users.list or smth, idk)

sir-sigurd commented 2 months ago

imo it would look nice if we had some sort of a namespace (e.g. admin.users.list or smth, idk)

I made this change