fortran-lang / registry

Registry for Fortran package manager
MIT License
8 stars 3 forks source link

fpm registry setup #3

Closed henilp105 closed 1 year ago

henilp105 commented 1 year ago

This PR sets up all the user login crud functions and the docker container with flask , nginx and mongodb.

Thanks and Regards, Henil

CC @awvwgk @fortran-lang/admins @fortran-lang/fpm

henilp105 commented 1 year ago

This PR sets up all the user login crud functions and the docker container with flask , nginx and mongodb.

Thanks and Regards, Henil

CC @awvwgk @fortran-lang/admins @fortran-lang/fpm @arteevraina @minhqdao @perazz

henilp105 commented 1 year ago

I have also added search , upload package , user main home page in this PR.

henilp105 commented 1 year ago

Thanks for the review and Sure sir, I have patched the login and signup templates. @everythingfunctional

milancurcic commented 1 year ago

Thanks, @henilp105. A few suggestions for the route layout:

  1. Auth-related endpoints (/signup, /login, /logout, /reset-password, /forgotpassword): Consider putting them under /auth/* root, i.e. /auth/signup, /auth/login, etc. It will make a more readable API.
  2. /forgotpassword -> /forgot-password for consistency with /reset-password.
  3. /<username>/* -> /users/<username>/* . More RESTful and avoid clashes with top-level routes (if any).
  4. Likewise for packages, i.e. consider GET:/search/<query> to be GET:/packages where different query parameters allow you to search for a subset of packages. GET:/packages without query parameters should return all packages in the registry.
  5. Instead of POST:/<username>/upload for uploading a package, POST:/packages is more restful and seems more natural to me.

In summary, something like this:

Some of these endpoints may be out of scope for this PR.

henilp105 commented 1 year ago

Thanks alot for the review, I will surely patch suggestions. @milancurcic

henilp105 commented 1 year ago

@arteevraina sure, I will be integrating the suggestions into the issues soon.

henilp105 commented 1 year ago

@milancurcic I could not get the use of

  1. GET:/users , we already have GET:/users/<username> , we would not want to show the name/details of all the users in a registry, it would be both computationally intensive and privacy concerning, have I misunderstood the use/significance of this route.
  2. GET:/packages , we would not want to show all the packages in the registry at once, it would be computationally intensive, may be we could rate limit it to some n packages at once.I had also been considering we should have some labels like trending packages, new releases section on / route.
everythingfunctional commented 1 year ago
  1. I think it's pretty common to be able to search/find users and see some information on their profile page. We could have a setting for users to choose not to be included in search results or have any information public on their profile page. In any case I think a GET request for the users route does make sense.
  2. It might be interesting to have a default ordering for this route besides alphabetical. Something like trending is a good idea.

For both routes, it definitely makes sense to paginate them, as you are correct, you don't want to be querying and sending the whole set of either. The behavior and queries for the pagination are pretty common, so should be easy to find in tutorials and docs. It usually makes sense to include query parameters in the URL like /users?page=3&sorted_by=joined&search_string=somebody