feat: add endpoints for upload, list and delete #1
Conversation
Reviewer's Guide by SourceryThis pull request introduces several new features and configurations to the project. It adds endpoints for file upload, listing, and deletion using Flask and Minio. Additionally, it includes various configuration files for documentation, code quality, testing, and continuous integration. File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @psankhe28 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Documentation: 3 issues found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
|
@psankhe28 dw about test and documentation CI, I'll fix that. for types, you need to install the types of the libraries you are using. Check the CI logs, or below. |
| @@ -0,0 +1,21 @@ | |||
| { | |||
| "template": "D:\\open-source\\gsoc\\cookiecutter-python", | |||
There was a problem hiding this comment.
@uniqueg we need to merge and finalize the cookiecutter, when generated via gh, this shoul dbe gh link, that is not possible before merge.
TusStorageHandler/minio_client.py
Outdated
|
|
||
| import os | ||
|
|
||
| from dotenv import load_dotenv |
There was a problem hiding this comment.
I personally think, using dotenv package, and letting python handle env variable is a good idea. I think environment variables should be handled by the "environment", expecting having a .env file with creds might be a bad idea.
What I do is I use either my bash/zsh{rc}, export the variable in the shell env or just use a tool that help me manage my env variables, like direnv. Not asking you to set them up, or change your workflow just think .env file might not be ideal imo.
thoughts @uniqueg ??
| tus_server = TusServer(minio_client, MINIO_BUCKET) | ||
|
|
||
|
|
||
| @app.route("/", methods=["GET"]) |
There was a problem hiding this comment.
If you're writing an API why not use FOCA??
There was a problem hiding this comment.
Yes, I plan to upgrade it using FOCA. First, I want to create a basic template to test it.
TusStorageHandler/requirements.txt
Outdated
There was a problem hiding this comment.
The contents of the requirements.txt file are not showing up in the list of modified files, but they are visible when viewing the file in the branch. I'm not sure why this is happening.
There was a problem hiding this comment.
you don't need requiements.txt, you should install the deps via poetry. Read the the guide.
|
use |
There was a problem hiding this comment.
Thanks a lot, @psankhe28. Unfortunately, this PR is way too big for us to review effectively. Please break it down into several smaller PRs:
- 1
chorefor.gitignore,CODE_OF_CONDUCT.md,PULL_REQUEST_TEMPLATE.mdetc. - 1
'cifor CI workflows - 1
docsfor docs stuff - 1
buildfor build system (pyproject.toml) - 3
featfor code (1 for each endpoint), each with unit tests and other corresponding changes, if applicable (e.g., doc/build updates); e.g., you don't wanna add dependencies (e.g.,tus) to your previousbuildPR that you don't need at the time of adding the build system; only add them to the PR where they are needed
|
See #3 (comment) and the comment above, @JaeAeich & @psankhe28 |
Summary by Sourcery
This pull request introduces new endpoints for file upload, list, and delete functionalities using Flask and Minio. It also adds a Makefile for managing various tasks, sets up GitHub Actions workflows for CI/CD, and configures Sphinx for documentation. Additionally, it includes initial test setups and various configuration files for code quality and security.