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

Add optional proxy to tus client & uploader #84

Merged
merged 2 commits into from
Mar 20, 2023
Merged

Add optional proxy to tus client & uploader #84

merged 2 commits into from
Mar 20, 2023

Conversation

pdenooijer
Copy link
Contributor

@pdenooijer pdenooijer commented Mar 7, 2023

By default no proxy is used, which will look up if it should use a proxy or a direct request. Adding the possibility to add a proxy instance pointing to the proxy to use when uploading.

This will fix #52.

README.md Outdated Show resolved Hide resolved
src/main/java/io/tus/java/client/TusUploader.java Outdated Show resolved Hide resolved
src/test/java/io/tus/java/client/TestTusClient.java Outdated Show resolved Hide resolved
Copy link
Member

@Acconut Acconut left a comment

Choose a reason for hiding this comment

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

Thank you for this PR! I have a few concerns but overall this goes in a great direction. Please also avoid force pushing, which makes reviews hard.

src/main/java/io/tus/java/client/TusUploader.java Outdated Show resolved Hide resolved
src/main/java/io/tus/java/client/TusClient.java Outdated Show resolved Hide resolved
src/main/java/io/tus/java/client/TusClient.java Outdated Show resolved Hide resolved
src/main/java/io/tus/java/client/TusClient.java Outdated Show resolved Hide resolved
@pdenooijer pdenooijer requested a review from Acconut March 8, 2023 12:49
@pdenooijer
Copy link
Contributor Author

pdenooijer commented Mar 8, 2023

Thank you for this PR! I have a few concerns but overall this goes in a great direction. Please also avoid force pushing, which makes reviews hard.

I added a new commit this time, that should fix all comments. On a side note, on a force push you could use the compare functionality for a review 🙂.

@pdenooijer
Copy link
Contributor Author

@Acconut I found a small inconsistency in the readme, should now be ready for review round two!

Copy link
Member

@Acconut Acconut left a comment

Choose a reason for hiding this comment

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

Thank you very much. This looks nearly finished. Just two minor touches are left.

README.md Show resolved Hide resolved
By default the NO_PROXY proxy is used which is the same as a direct
request. Adding the possibility to override the proxy with an own
instance pointing to the proxy to use when uploading.
Copy link
Member

@Acconut Acconut left a comment

Choose a reason for hiding this comment

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

Thank you very much!

@Acconut Acconut merged commit 67de05c into tus:main Mar 20, 2023
@pdenooijer pdenooijer deleted the task/add_optional_proxy branch March 20, 2023 13:37
@pdenooijer
Copy link
Contributor Author

Awesome!

@Acconut
Copy link
Member

Acconut commented Mar 20, 2023

Just released https://github.com/tus/tus-java-client/releases/tag/0.5.0.

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.

How to set Proxy for TUSClient?
3 participants