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

API for deleting package. #9

Closed
wants to merge 34 commits into from

Conversation

arteevraina
Copy link
Member

@arteevraina arteevraina commented Dec 27, 2022

I have created the PR for initializing the API for deleting a package. Though, I have not used the original API route that was discussed in #5 .

I had an idea to use this route.

/packages/<username>/<namespace_name>/<package_name>/delete 

instead of

/<username>/<package_id>/delete

because I was doubtful that _id of the mongo DB document would be correctly sent by the URL.

Commits to review:

@henilp105 @minhqdao @perazz @fortran-lang/fpm I have few questions here, Since we have nothing merged in the main branch as of today And this PR contains changes made by @henilp105 as well since the start of project. Should I consider creating this PR to the henils remote branch and can be reviewed there as of now because it will be easier to review then.

@henilp105
Copy link
Member

@arteevraina I had considered package_name in the field maybe it was incurred due to typo. I am considering package_name as the field in the API.

1 similar comment
@henilp105
Copy link
Member

@arteevraina I had considered package_name in the field maybe it was incurred due to typo. I am considering package_name as the field in the API.

@minhqdao
Copy link
Contributor

To answer your question: The ideal case would be that PRs are quickly merged into main and new PRs are also opened and merged against main. That and keeping PR sizes small should result in a small number of merge conflicts and short development cycles.

@henilp105
Copy link
Member

@arteevraina I have made the flask code modular which would make it easy to add new features/commits and also prevent (mostly) the merge conflicts.

@arteevraina arteevraina force-pushed the delete-package branch 2 times, most recently from 08451f9 to 9c639ca Compare January 2, 2023 18:00
def delete_package(username, namespace_name, package_name):
uuid = request.cookies.get("uuid")
if not uuid:
return render_template("login.html")
Copy link
Member

Choose a reason for hiding this comment

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

@arteevraina We would have to add a json response of unauthorized 403, as we have shifted to react/next.js frontend and for CLI application.

flask/packages.py Outdated Show resolved Hide resolved
flask/packages.py Outdated Show resolved Hide resolved
arteevraina pushed a commit to arteevraina/registry that referenced this pull request Feb 4, 2023
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.

3 participants