Smart_open: Exception thrown when parsing URI without "@"

Created on 19 Sep 2018  路  4Comments  路  Source: RaRe-Technologies/smart_open

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:

bug good first issue

Most helpful comment

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

All 4 comments

@rileypeterson hello, submit PR are the pretty good idea :+1:

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

Fixed by #235

Was this page helpful?
0 / 5 - 0 ratings

Related issues

michelazzo picture michelazzo  路  3Comments

vuryleo picture vuryleo  路  6Comments

eschwartz picture eschwartz  路  5Comments

ivanhigueram picture ivanhigueram  路  5Comments

jayantj picture jayantj  路  3Comments