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

feat: Add filesystem and projects API #389

Merged
merged 10 commits into from
Aug 22, 2023

Conversation

gjreda
Copy link
Collaborator

@gjreda gjreda commented Aug 21, 2023

fixes #365


This PR adds APIs for both managing files and projects.

Running the server

cd python; poetry run uvicorn web:api --reload

Project API

Create a project

curl -X 'POST' \
  'http://127.0.0.1:8000/api/projects/' \
  -H 'accept: application/json' \
  -d '' -s | jq

{
  "63f40666-6aae-49c6-b375-be09ca3dd5bc": "/tmp/web-storage-url/user1/63f40666-6aae-49c6-b375-be09ca3dd5bc"
}

List projects

curl -X 'GET' \
  'http://127.0.0.1:8000/api/projects/' \
  -H 'accept: application/json' -s | jq

{
  "project1": "/tmp/web-storage-url/user1/project1",
  "project2": "/private/var/folders/4v/0m97cs6j7v53r6m3mwg340380000gn/T/pytest-of-greg/pytest-389/test_update_project_path_stora1/user1/project2",
  "d94ca1db-f251-48c3-aafe-67be329b6006": "/tmp/web-storage-url/user1/d94ca1db-f251-48c3-aafe-67be329b6006",
  "63f40666-6aae-49c6-b375-be09ca3dd5bc": "/tmp/web-storage-url/user1/63f40666-6aae-49c6-b375-be09ca3dd5bc"
}

Get project details

curl -X 'GET' \
  'http://127.0.0.1:8000/api/projects/63f40666-6aae-49c6-b375-be09ca3dd5bc' \
  -H 'accept: application/json' -s | jq

{
  "project_id": "63f40666-6aae-49c6-b375-be09ca3dd5bc",
  "project_path": "/tmp/web-storage-url/user1/63f40666-6aae-49c6-b375-be09ca3dd5bc",
  "filepaths": []
}

Delete a project

curl -X 'DELETE' \
  'http://127.0.0.1:8000/api/projects/63f40666-6aae-49c6-b375-be09ca3dd5bc' \
  -H 'accept: application/json'

{
  "status": "success",
  "message": "Project deleted",
  "project_id": "63f40666-6aae-49c6-b375-be09ca3dd5bc"
}

List project files

curl -X 'GET' \
  'http://127.0.0.1:8000/api/projects/63f40666-6aae-49c6-b375-be09ca3dd5bc/files' \
  -H 'accept: application/json'

Filesystem API

Create a file (this will respect directory segments)

curl -X 'PUT' \
  'http://127.0.0.1:8000/api/fs/b532a8c1-8bea-47e2-9f2f-0ac93e4da623/uploads%2Ftest.pdf' \
  -H 'accept: application/json' \
  -H 'Content-Type: multipart/form-data' \
  -F 'file=@A Few Useful Things to Know about Machine Learning.pdf;type=application/pdf'

{
  "status": "success",
  "message": "File uploaded",
  "filepath": "/tmp/web-storage-url/user1/b532a8c1-8bea-47e2-9f2f-0ac93e4da623/uploads/test.pdf"
}

Read a file

curl -X 'GET' \
  'http://127.0.0.1:8000/api/fs/b532a8c1-8bea-47e2-9f2f-0ac93e4da623/uploads%2Ftest.pdf' \
  -H 'accept: application/json'

Delete a file

curl -X 'DELETE' \
  'http://127.0.0.1:8000/api/fs/b532a8c1-8bea-47e2-9f2f-0ac93e4da623/uploads%2Ftest.pdf' \
  -H 'accept: application/json'

{
  "status": "success",
  "message": "File deleted",
  "filepath": "/tmp/web-storage-url/user1/b532a8c1-8bea-47e2-9f2f-0ac93e4da623/uploads/test.pdf"
}

@gjreda gjreda changed the title POC filesystem api feat: Add filesystem and projects API Aug 21, 2023
@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Merging #389 (5249f2a) into main (fe1a3dc) will increase coverage by 0.11%.
Report is 1 commits behind head on main.
The diff coverage is 87.94%.

❗ Current head 5249f2a differs from pull request most recent head f026603. Consider uploading reports for the commit f026603 to get more accurate results

@@            Coverage Diff             @@
##             main     #389      +/-   ##
==========================================
+ Coverage   84.10%   84.21%   +0.11%     
==========================================
  Files         174      176       +2     
  Lines        9964    10135     +171     
  Branches     1097     1101       +4     
==========================================
+ Hits         8380     8535     +155     
- Misses       1573     1589      +16     
  Partials       11       11              
Files Changed Coverage Δ
python/sidecar/http.py 84.68% <83.14%> (-3.96%) ⬇️
python/sidecar/projects.py 95.91% <95.91%> (ø)
python/sidecar/settings.py 100.00% <100.00%> (ø)
python/web.py 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

# Filesystem API
# --------------
@filesystem_api.put("/{project_id}/{filepath:path}")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cguedes This will respect the filepath provided, so uploading a named foo.pdf to uploads/test.pdf will result in the appropriate directory being created and test.pdf being written there.

I think this is what we were discussing this morning, but let me know otherwise.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, this is exactly what we want.

filepath = project_path / filepath
try:
os.remove(filepath)
Copy link
Collaborator Author

@gjreda gjreda Aug 21, 2023

Choose a reason for hiding this comment

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

@cguedes I ended up deleting filesystem.py because once I cleaned this PR up, this line was literally the only one remaining that was actually used.

FastAPI gave us a lot for free with FileResponse and UploadFile.

@gjreda gjreda marked this pull request as ready for review August 21, 2023 22:16
@gjreda
Copy link
Collaborator Author

gjreda commented Aug 21, 2023

There was a lot of code to knock out today and there's probably some clean up necessary, but I think it's better to get the functionality in place first to unblock others. Any clean up can happen in subsequent PRs.

@gjreda gjreda requested a review from cguedes August 22, 2023 00:42
@cguedes
Copy link
Collaborator

cguedes commented Aug 22, 2023

There was a lot of code to knock out today and there's probably some clean up necessary, but I think it's better to get the functionality in place first to unblock others. Any clean up can happen in subsequent PRs.

Great work @gjreda I've just added a commit to ensure that the server path exists (/tmp/....) and some code cleanup for the paths.

Also, move the projects.json file under the user's folder instead of using projects_{user_id}.json.

There is still work to do, for example in the response payload for the project files (not sure if it should be a list of files/directories vs nested structure), but this def unblock the frontend work for the web.

@cguedes cguedes merged commit 4516f86 into main Aug 22, 2023
8 of 9 checks passed
@cguedes cguedes deleted the 365-create-pythonhttp-equivalent-of-filesystemts branch August 22, 2023 09:23


@filesystem_api.get("/{project_id}/{filepath:path}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there anything that prevents reading from outside the project path here? Could filepath be something like ../../../some-other-folder or /etc/passwd? Just wondering about security here.

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.

Create Python/HTTP equivalent of filesystem.ts
3 participants