Refactor/reorg #64
Refactor/reorg #64
Conversation
This adds a simple FHS-based Nix development environment for the project.
|
I think it makes sense the uuid options preflight call, can't remember why I added, but can be removed. Thanks for the great work! |
ixxie
force-pushed
the
refactor/reorg
branch
2 times, most recently
from
15e1951 to
437219a
Compare
Thank you for your confidence, but the branch was broken when you approved it. Its good if you actually test it when reviewing, so that we catch bugs before they go to production. I believe its working now, but I need to still finalize the example code. |
ixxie
force-pushed
the
refactor/reorg
branch
3 times, most recently
from
4fc09a8 to
7c2fb7a
Compare
|
@edihasaj I think it should be ready now; can you please test it? I updated the README to explain how to run the updated example. FYI the reason I modified the example is that I plan to expand it with an Nginx reverse proxy and dockerize it. So my goal is to prepare the example too look more like a real-world production example. |
This seperates several low-level APIs from the high-level routing: - TusUploadFile (file.py): class for the upload file itself - TusUploadInfo (info.py): class for the file's .info param file - TusUploadParams (params.py): pydantic model for upload params Additionally, some helper functions are moved to a request.py file, and routes are seperated into distinct files for the core protocols and its extensions. Finally, deprecated routes are removed. The motivation is to improve understandibility of the codebase, and facilitate maintainance going forward.
|
I will test it during the weekend, looks good but since it is a breaking change, and I use it in production already can't risk changing without testing in staging envs first.. thanks for the good work |
Absolutely, every PR should be thoroughly tested by the reviewer IMO. Even non-breaking changes.
You are very welcome! |
|
p.s. I made a PR to the TUS protocol to add an table that gives an overview of the protocol. I hope to use it to review |
|
@edihasaj thanks for reviewing and merging. Should we release this also? |
|
not yet, I noticed some issues with location headers but couldn't check |
What kind of issues? |
|
based on the current setup the path was not working with some prefix slash / in the back and / at the end, then after the first POST call, the location URL makes protocol::/host:port/guid instead of protocol::/host:port/prefix/guid. I was checking them but couldn't find some more time to put an end to it yet, will try on the weekend |
|
I think I am confused now, it works when on staging but not locally (maybe I have been having something wrong with my setup) but I will run some more tests and then make it a stable release 4.0.0 |
|
this one is now merged to production and released to 4.0.0. appreciate your good work! |
Good to hear! |
2 participants
Continuation of #63
The previous PR was terminated due to a git mishap.