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

Exception thrown when parsing URI without "@" #234

Closed
rileypeterson opened this issue Sep 19, 2018 · 4 comments
Closed

Exception thrown when parsing URI without "@" #234

rileypeterson opened this issue Sep 19, 2018 · 4 comments

Comments

@rileypeterson
Copy link
Contributor

rileypeterson commented Sep 19, 2018

Recently had some test failures caused by smart_open.smart_open. I'm pretty sure this is due to the pull requests that were accepted from Issue 94 and this line.

Here is the traceback from Jenkins:

        # Common URI template [secret:key@][host[:port]@]bucket/object
        try:
            uri = parsed_uri.netloc + parsed_uri.path
            # Separate authentication from URI if exist
            if ':' in uri.split('@')[0]:
>               auth, uri = uri.split('@', 1)
E               ValueError: not enough values to unpack (expected 2, got 1)

In my instance this variable:
uri = parsed_uri.netloc + parsed_uri.path
contains no @ symbol but does contain :

Thus there aren't enough values to unpack on the split:

uri = 'our-bucket/our/lengthy/file_path/with/a/date/in/it/2018:08:01'

if ":" in uri.split('@')[0]:
    auth, uri = uri.split('@', 1)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-58-fc3bf5a8bea2> in <module>()
      1 if ":" in uri.split('@')[0]:
----> 2     auth, uri = uri.split('@', 1)

ValueError: not enough values to unpack (expected 2, got 1)

Should I submit a PR?

I think all that might need to be done is:

if ":" in uri.split('@')[0]: ---> if ":" in uri.split('@')[0] and '@' in uri:

@menshikh-iv
Copy link
Contributor

@rileypeterson hello, submit PR are the pretty good idea 👍

@piskvorky
Copy link
Owner

Don't forget to include a unit test (fails before, passes after), to avoid regressions.

@rileypeterson
Copy link
Contributor Author

Here is the PR

@menshikh-iv
Copy link
Contributor

Fixed by #235

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants