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

Allow trailing slashes in SDK client url #5057

Merged
merged 4 commits into from
Oct 9, 2022

Conversation

zhiltsov-max
Copy link
Contributor

Motivation and context

Closes #5046

  • Allowed trailing slashes in Client(url=) and make_client(host=) of CVAT SDK

How has this been tested?

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.

@SpecLad
Copy link
Contributor

SpecLad commented Oct 7, 2022

For URLs this makes sense, but I don't think we should be doing this for hosts. Slashes are not valid in host names.

@zhiltsov-max
Copy link
Contributor Author

Slashes are not valid in host names.

Hm, I don't think the majority of users know about this, and even it they do, it's still very easy to make this mistake.

Copy link
Contributor

@SpecLad SpecLad left a comment

Choose a reason for hiding this comment

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

OK, it looks like the host parameter basically already accepts full URLs, so might as well.

@zhiltsov-max
Copy link
Contributor Author

@nmanovic , this PR is ready for merging.

@nmanovic nmanovic merged commit 511f970 into develop Oct 9, 2022
@nmanovic nmanovic deleted the zm/allow-trailing-slash-in-sdk-url branch October 9, 2022 07:01
@nmanovic nmanovic mentioned this pull request Dec 12, 2022
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.

An additional slash at the end of the hostname makes some operations unworkable
3 participants