Closed arteevraina closed 1 year ago
This has been completed and open for a quick review.
There is a users
collection and then a new collection is created for every user
? Looks unusual to me. Why isn't a user
a document in the users
collection? Or is this a mistake or am I reading this wrongly?
Same here, shouldn't be namespace
a document in the namespaces
collection?
When I use the api to obtain package maintainers, I get a list all the maintainers including their internal ids. Is that safe? Shouldn't you only be able to obtain a list of maintainers when you are logged in as the a package maintainer/namespace maintainer/namespace admin/admin?
Using the uuid from above, I tried deleting my namespace and got a code 200, which is being described as "Namespace deleted successfully". It yet didn't work, however I'm receiving an html body as the response body instead of the usual JSON response with code and message. That does not correlate to the documentation and also would be quite inconvenient to handle if, for example, called from a program, such as a CLI tool.
Same for deleting a package.
Forgot password also returns an html instead of a JSON.
The "Execute" button doesn't work when trying out the logout api.
There is a
users
collection and then a new collection is created for everyuser
? Looks unusual to me. Why isn't auser
a document in theusers
collection? Or is this a mistake or am I reading this wrongly?
There is always a single collection i.e there is users
collection which holds all the registry users, packages
collection which holds all the packages and a namespaces
collection which holds all the namespaces.
So, in the endpoint {username}/namespace/admin
. It does not mean that we are fetching it from a unique user collection.
Same here, shouldn't be
namespace
a document in thenamespaces
collection?
Yes, it exactly is. Endpoints can be named anything (although domain specific) but it does not implies that we are bound to write collection name first.
When I use the api to obtain package maintainers, I get a list all the maintainers including their internal ids. Is that safe? Shouldn't you only be able to obtain a list of maintainers when you are logged in as the a package maintainer/namespace maintainer/namespace admin/admin?
The internal id's are the document id's and they cannot be used to access the authorized endpoints of the registry so it's completely safe.
Coming to your second question, I kept the frontend check only for the condition you mentioned. But, I guess it will be nice to have it inside the API as well.
I would create an issue for this and handle that change in a separate PR because that is out of scope of this PR.
Using the uuid from above, I tried deleting my namespace and got a code 200, which is being described as "Namespace deleted successfully". It yet didn't work, however I'm receiving an html body as the response body instead of the usual JSON response with code and message. That does not correlate to the documentation and also would be quite inconvenient to handle if, for example, called from a program, such as a CLI tool.
A normal user cannot delete a namespace or a package. It is to be done by sudo admins for which a user is supposed to signup with a key password that is defined in environment variables.
You are getting this "Internal Server Error" because backend is crashing when this API is called and I know why this is happening (null pointer exception). @henilp105 ?
There is a
users
collection and then a new collection is created for everyuser
? Looks unusual to me. Why isn't auser
a document in theusers
collection? Or is this a mistake or am I reading this wrongly?There is always a single collection i.e there is
users
collection which holds all the registry users,packages
collection which holds all the packages and anamespaces
collection which holds all the namespaces.So, in the endpoint
{username}/namespace/admin
. It does not mean that we are fetching it from a unique user collection.
I still don't know what's the full endpoint.
Why not calling it /users/{username}/namespace/admin
for clarification then (if it's that)?
- Reset password API
- Execute button in logout api both should be fixed now. @minhqdao
Can't you use a variable/constant for the base url that is later swapped for the production one so we are always showing a valid url?
Apis for:
1) Deleting namespace 2) Deleting package 3) Deleting version (?)
are still crashing.
Apis for:
- Deleting namespace
- Deleting package
- Deleting version (?)
are still crashing.
@minhqdao you would have to enter the uuid
of the user to delete them ( they can only be deleted by the sudo admins) , not the token
for a namespace.
Can't you use a variable/constant for the base url that is later swapped for the production one so we are always showing a valid url?
We are using an env variable for this , maybe @arteevraina forgot to set the env var thus this none is coming.
You are getting this "Internal Server Error" because backend is crashing when this API is called and I know why this is happening (null pointer exception). @henilp105 ?
@arteevraina It mostly seems to happen if we haven't predefined/set some variable but there is an error handler implemented so None gets set but during access of that var it breaks things.
Apis for:
- Deleting namespace
- Deleting package
- Deleting version (?)
are still crashing.
@minhqdao you would have to enter the
uuid
of the user to delete them ( they can only be deleted by the sudo admins) , not thetoken
for a namespace.
So can we set it up so that the backend doesn't crash when we enter no or a wrong uuid
?
Apis for:
- Deleting namespace
- Deleting package
- Deleting version (?)
are still crashing.
@minhqdao you would have to enter the
uuid
of the user to delete them ( they can only be deleted by the sudo admins) , not thetoken
for a namespace.So can we set it up so that the backend doesn't crash when we enter no or a wrong
uuid
?
Should be fixed now.
I still don't know what's the full endpoint.
Currently the full endpoint is this only : /{username}/namspace/admin
Why not calling it
/users/{username}/namespace/admin
for clarification then (if it's that)?
Yes, ofcourse we can call it this. Should be done in a separate PR.
@minhqdao @henilp105 If all the documentation based issues are resolved in the PR, I think we can merge it after a final review. Rest all issues we an handle in small separate PRs.
Can't you use a variable/constant for the base url that is later swapped for the production one so we are always showing a valid url?
This should also be resolved.
Description
Related Issues
11
How to Test
For testing you can use this link. And go through the documentation and even try to run the APIs by supplying necessary information.
Link for Testing
https://registry-apis.vercel.app/apidocs/#/