Skip to content

Refactor/reorg - #64

Merged
edihasaj merged 14 commits into
edihasaj:mainfrom
ixxie:refactor/reorg
Jul 1, 2025
Merged

edihasaj merged 14 commits into
edihasaj:mainfrom
ixxie:refactor/reorg

Conversation

@ixxie

@ixxie ixxie commented Jun 23, 2025

Copy link
Copy Markdown
Collaborator

Continuation of #63

The previous PR was terminated due to a git mishap.

@edihasaj

Copy link
Copy Markdown
Owner

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
ixxie force-pushed the refactor/reorg branch 2 times, most recently from 15e1951 to 437219a Compare June 24, 2025 12:55
@ixxie

ixxie commented Jun 24, 2025

Copy link
Copy Markdown
Collaborator Author

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!

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
ixxie force-pushed the refactor/reorg branch 3 times, most recently from 4fc09a8 to 7c2fb7a Compare June 24, 2025 13:39
@ixxie
ixxie requested a review from edihasaj June 24, 2025 13:42
@ixxie

ixxie commented Jun 24, 2025

Copy link
Copy Markdown
Collaborator Author

@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.

ixxie added 5 commits June 25, 2025 15:58
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.
@edihasaj

Copy link
Copy Markdown
Owner

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

@ixxie

ixxie commented Jun 26, 2025

Copy link
Copy Markdown
Collaborator Author

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..

Absolutely, every PR should be thoroughly tested by the reviewer IMO. Even non-breaking changes.

thanks for the good work

You are very welcome!

@ixxie

ixxie commented Jun 26, 2025

Copy link
Copy Markdown
Collaborator Author

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 tuspyserver after this PR lands, and verify everything is correctly implemented. I already spotted one potential issue.

@edihasaj
edihasaj merged commit caa2725 into edihasaj:main Jul 1, 2025
@ixxie

ixxie commented Jul 3, 2025

Copy link
Copy Markdown
Collaborator Author

@edihasaj thanks for reviewing and merging. Should we release this also?

@edihasaj

edihasaj commented Jul 4, 2025

Copy link
Copy Markdown
Owner

not yet, I noticed some issues with location headers but couldn't check

@ixxie

ixxie commented Jul 4, 2025

Copy link
Copy Markdown
Collaborator Author

not yet, I noticed some issues with location headers but couldn't check

What kind of issues?

@edihasaj

edihasaj commented Jul 4, 2025

Copy link
Copy Markdown
Owner

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

@edihasaj

edihasaj commented Jul 5, 2025

Copy link
Copy Markdown
Owner

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

@edihasaj

edihasaj commented Jul 5, 2025

Copy link
Copy Markdown
Owner

this one is now merged to production and released to 4.0.0.

appreciate your good work!

@ixxie

ixxie commented Jul 5, 2025

Copy link
Copy Markdown
Collaborator Author

this one is now merged to production and released to 4.0.0.

appreciate your good work!

Good to hear!

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.

2 participants