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

[feature] transition the package_roles method onto the restful API #9770

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

4383
Copy link

@4383 4383 commented Jul 8, 2021

These changes adds the package_roles as results of the return of the admin.views.project restful API.

The package_roles method was available into the xmlrpc, unfortunately these informations are missing into the restful API [1].

These changes adds the creation of an equivalent into the restful API.

[1] #9700

These changes adds the `package_roles` as results of the return of the
`admin.views.project` restful API.

The `package_roles` method was available into the xmlrpc, unfortunately
these informations are missing into the restful API [1].

These changes adds the creation of an equivalent into the restful API.

[1] pypi#9700
@@ -99,6 +99,7 @@ def test_gets_project(self, db_request):
"project": project,
"releases": [],
"maintainers": roles,
"package_roles": [],
Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure if we want to create some representations by using the RoleFactory. In this case what should be the difference between maintainers and package_roles?

@4383
Copy link
Author

4383 commented Jul 8, 2021

These changes aim to address #9700

@4383
Copy link
Author

4383 commented Aug 4, 2021

Hello team,

I hate harassing people for getting reviews, but, at least, can we have a minimal analyse from your side just to know if this is the right thing to do to fix #9700 and get some visibility on our side.

Thanks for your understanding.

@abitrolly
Copy link
Contributor

abitrolly commented Aug 31, 2021

JSON API at https://warehouse.readthedocs.io/api-reference/json.html#get--pypi--project_name--json promises this.

Metadata returned comes from the values provided at upload time and does not necessarily match the content of the uploaded

And package_roles is not a part of these values.

I don't think it is a good idea to skew the data that goes into [packaging] -> [uploading] -> [storing] -> [accessing through API] pipelines without documenting all the transformations that may come unexpected for people. I mean #3520 needs to be merged first to solve [packaging] -> [uploading] uncertainty in data fields and values, and then a similar mapping will need to be done to decouple fields in REST API from what people have in packages and uploads.

Or, make a separate namespace that will expose PyPI admin interface for a package, allowing to assign maintainership roles etc. Right now PyPI evolution stuck between going into central command and control server and providing a more decentralized alternative. When all information including maintainers was contained in the package, the management was decentralized. When people started to need the account and configure access on the server side, the management is centralized.

Given the above, I would not just make a separate namespace in the JSON, like "pypi_rbac" or "pypi_config", but a separate endpoint like api/v2/config/<project_name>/access, which will only manage PyPI settings. Because you would likely need the API to limit role scopes, assign token, check their validity, audit logs.

I mean there is a lot of signals that the admin API is needed https://github.com/pypa/warehouse/issues?q=is%3Aissue+is%3Aopen+token+API

@4383
Copy link
Author

4383 commented Sep 2, 2021

Thanks for your interesting feedback

@4383 4383 requested a review from a team as a code owner February 22, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants