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

fails on extras syntax #70

Closed
tmc opened this issue Jul 14, 2018 · 12 comments
Closed

fails on extras syntax #70

tmc opened this issue Jul 14, 2018 · 12 comments
Assignees

Comments

@tmc
Copy link

tmc commented Jul 14, 2018

example:

$ hashin 'python-language-server[yapf]'
Traceback (most recent call last):
  File "/home/user/go/src/github.com/user/flaskexp/ve/bin/hashin", line 11, in <module>
    sys.exit(main())
  File "/home/user/go/src/github.com/user/flaskexp/ve/lib/python3.6/site-packages/hashin.py", line 499, in main
    include_prereleases=args.include_prereleases,
  File "/home/user/go/src/github.com/user/flaskexp/ve/lib/python3.6/site-packages/hashin.py", line 129, in run
    run_single_package(spec, *args, **kwargs)
  File "/home/user/go/src/github.com/user/flaskexp/ve/lib/python3.6/site-packages/hashin.py", line 157, in run_single_package
    include_prereleases=include_prereleases,
  File "/home/user/go/src/github.com/user/flaskexp/ve/lib/python3.6/site-packages/hashin.py", line 429, in get_package_hashes
    data = get_package_data(package, verbose)
  File "/home/user/go/src/github.com/user/flaskexp/ve/lib/python3.6/site-packages/hashin.py", line 358, in get_package_data
    content = json.loads(_download(url))
  File "/home/user/go/src/github.com/user/flaskexp/ve/lib/python3.6/site-packages/hashin.py", line 100, in _download
    r = urlopen(url)
  File "/usr/lib/python3.6/urllib/request.py", line 223, in urlopen
    return opener.open(url, data, timeout)
  File "/usr/lib/python3.6/urllib/request.py", line 532, in open
    response = meth(req, response)
  File "/usr/lib/python3.6/urllib/request.py", line 642, in http_response
    'http', request, response, code, msg, hdrs)
  File "/usr/lib/python3.6/urllib/request.py", line 564, in error
    result = self._call_chain(*args)
  File "/usr/lib/python3.6/urllib/request.py", line 504, in _call_chain
    result = func(*args)
  File "/usr/lib/python3.6/urllib/request.py", line 756, in http_error_302
    return self.parent.open(new, timeout=req.timeout)
  File "/usr/lib/python3.6/urllib/request.py", line 532, in open
    response = meth(req, response)
  File "/usr/lib/python3.6/urllib/request.py", line 642, in http_response
    'http', request, response, code, msg, hdrs)
  File "/usr/lib/python3.6/urllib/request.py", line 564, in error
    result = self._call_chain(*args)
  File "/usr/lib/python3.6/urllib/request.py", line 504, in _call_chain
    result = func(*args)
  File "/usr/lib/python3.6/urllib/request.py", line 756, in http_error_302
    return self.parent.open(new, timeout=req.timeout)
  File "/usr/lib/python3.6/urllib/request.py", line 532, in open
    response = meth(req, response)
  File "/usr/lib/python3.6/urllib/request.py", line 642, in http_response
    'http', request, response, code, msg, hdrs)
  File "/usr/lib/python3.6/urllib/request.py", line 570, in error
    return self._call_chain(*args)
  File "/usr/lib/python3.6/urllib/request.py", line 504, in _call_chain
    result = func(*args)
  File "/usr/lib/python3.6/urllib/request.py", line 650, in http_error_default
    raise HTTPError(req.full_url, code, msg, hdrs, fp)
urllib.error.HTTPError: HTTP Error 404: Not Found
@peterbe
Copy link
Owner

peterbe commented Aug 8, 2018

Yeah, unfortunately, it means you have to do hashin python-language-server yapf.

@peterbe peterbe closed this as completed Aug 8, 2018
@di
Copy link
Contributor

di commented Oct 23, 2018

Just encountered this myself, I also assumed I could do hashin project[extra]. I think this wouldn't be too hard to add, any reason we can't support this?

@mythmon
Copy link
Contributor

mythmon commented Oct 23, 2018

One issue is that hashin doesn't resolve any dependencies. The only difference between python-language-server and python-language-server[yapf] is the dependencies. Since hashin only deals with the packages specified on the command line and doesn't go further, it doesn't make a lot of sense to me to specify extra dependencies.

That being said, it would be amazing if hashin supported resolving dependencies, and at that point it would be nice to support the extras syntax, and probably an easy additional feature.

@di
Copy link
Contributor

di commented Oct 23, 2018

Not sure I follow. In the example above, "yapf" is an just an extra whose name happens to correspond to the yapf package. It doesn't actually mean that yapf is a dependency at all (although I'm guessing it is), and it might mean that there are additional sub-dependencies that get installed when the "yapf" extra is used.

Regardless of the additional dependencies that may or may not be installed, it does make sense to allow the user to specify extras. For example, this is the current behavior:

$ hashin requests
$ cat requirements.txt
requests==2.20.0 \
    --hash=sha256:99dcfdaaeb17caf6e526f32b6a7b780461512ab3f1d992187801694cba42770c \
    --hash=sha256:a84b8c9ab6239b578f22d1c21d51b696dcfe004032bb80ea832398d6909d7279

This is the desired behavior:

$ hashin requests[security]
$ cat requirements.txt
requests[security]==2.20.0 \
    --hash=sha256:99dcfdaaeb17caf6e526f32b6a7b780461512ab3f1d992187801694cba42770c \
    --hash=sha256:a84b8c9ab6239b578f22d1c21d51b696dcfe004032bb80ea832398d6909d7279

This is not the same as:

$ hashin requests security
$ cat requirements.txt
requests==2.20.0 \
    --hash=sha256:99dcfdaaeb17caf6e526f32b6a7b780461512ab3f1d992187801694cba42770c \
    --hash=sha256:a84b8c9ab6239b578f22d1c21d51b696dcfe004032bb80ea832398d6909d7279
security==1.0.0 \
    --hash=sha256:whatever

@mythmon
Copy link
Contributor

mythmon commented Oct 23, 2018

Ah, thinking about this a bit more I understand what's going on now. I actually ran into this recently. In the above case, listing python-language-server[yapf] means that python-language-server is a top level dependency, and that yapf is a sub-dependencies (that would go in constraints.txt). Listing both python-language-server and yapf separately makes them both top level dependencies.

@peterbe I think there is benefit in this feature. Hashin would only use the package part, and largely ignore the extra part, except to persist it into the requirements file. I could have used this when adding GCP support to Normandy for example. I think you should re-open this issue.

@di
Copy link
Contributor

di commented Oct 23, 2018

In the above case, listing python-language-server[yapf] means that python-language-server is a top level dependency, and that yapf is a sub-dependencies (that would go in constraints.txt)

If this is true it's merely a coincidence, listing package[extra] does not necessarily mean that extra is a sub-dependency. See my example above.

@mythmon
Copy link
Contributor

mythmon commented Oct 23, 2018

Yes, sorry. I was trying to draw a distinction between hashin python-language-server[yapf] and hashin python-language-server yapf from the previous comment.

@peterbe
Copy link
Owner

peterbe commented Oct 24, 2018

Easy. We just treat the package name as pypi_lookup(name.split('[')).

I do worry that people might get false hopes from it though. For example, if you do hashin "python-language-server[yapf]" the user might think that hashin takes care of the yapf too (and into the same requirements file).

I guess we'll have to deal with that worry at some point but that's not now.

Today, if I want requests[security] I have to do do:

$ hashin -r reqs.txt requests
$ hashin -r constraints.txt cryptography PyOpenSSL urllib3 idna
$ emacs reqs.txt  # and replace 'requests' with 'requests[security]` 

If it would just be:

$ hashin -r reqs.txt "requests[security]"
$ hashin -r constraints.txt cryptography PyOpenSSL urllib3 idna

...it would be nice. I can take care of that.

@di
Copy link
Contributor

di commented Oct 24, 2018

I'd recommend using the packaging library for this instead:

>>> from packaging.requirements import Requirement
>>> req = Requirement("requests[security]")
>>> req.name
'requests'
>>> req.extras
{'security'}

@peterbe
Copy link
Owner

peterbe commented Oct 24, 2018

Awesome catch. I would have actually gone ahead with doing a home-made string trick on it myself. packaging is already a dependency of hashin so it's a slam dunk of an idea.

@peterbe peterbe reopened this Oct 24, 2018
@peterbe peterbe self-assigned this Oct 24, 2018
@peterbe
Copy link
Owner

peterbe commented Oct 24, 2018

I'd recommend using the packaging library for this instead:

Bummer that req.extras is a set. Now I can't know what order they were written in. For example:

>>> from packaging.requirements import Requirement
>>> req = Requirement("requests[security,additional]")
>>> req.extras
{'additional', 'security'}
>>> ','.join(req.extras)
'additional,security'

Do you know if there's a strong reason for that to be a set?

peterbe added a commit that referenced this issue Oct 24, 2018
@di
Copy link
Contributor

di commented Oct 24, 2018

Do you know if there's a strong reason for that to be a set?

Because the order doesn't matter. 🙂

peterbe pushed a commit that referenced this issue Oct 25, 2018
* allow extras syntax

Fixes #70

* use Requirement().str() instead
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

4 participants