uptane / obsolete-reference-implementation

⚠️⚠️⚠️ Obsolete ⚠️⚠️⚠️ — This repository contains a reference implementation of a pre-1.0 version of the Uptane Standard and should not be used
https://uptane.github.io
MIT License
6 stars 6 forks source link

Should `simplejson` be listed as a dependency? #27

Closed awwad closed 7 years ago

awwad commented 7 years ago

From @vladimir-v-diaz on March 8, 2017 17:29

I was following the instructions for the primary client (https://github.com/awwad/uptane#window-4-the-primary-clients) and encountered the following exception:

>>> import demo.demo_primary as dp
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "demo/demo_primary.py", line 34, in <module>
    import canonicaljson
  File "/home/vlad/projects/vladforks/tmp/uptane/env/local/lib/python2.7/site-packages/canonicaljson.py", line 18, in <module>
    import simplejson as json
ImportError: No module named simplejson

Copied from original issue: awwad/uptane#36

awwad commented 7 years ago

From @vladimir-v-diaz on March 8, 2017 17:34

frozendict also appears to be a required dependency (after I manually installed simplejson).

>>> import demo.demo_primary as dp
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "demo/demo_primary.py", line 34, in <module>
    import canonicaljson
  File "/home/vlad/projects/vladforks/tmp/uptane/env/local/lib/python2.7/site-packages/canonicaljson.py", line 20, in <module>
    from frozendict import frozendict
ImportError: No module named frozendict

I can successfully run the primary client after installing simplejson and frozendict. Not sure if this is an issue with my pip / setuptools installation...

awwad commented 7 years ago

I'm not sure. It might be your environment.

I'm a bit surprised that the import simplejson directive isn't couched in a try/except with import json as the fallback. simplejson has been in the std lib as json for a long while now, and json is a perfectly suitable fallback. :P

Anyway, oddly, I've never run into either error nor have I manually installed frozendict or simplejson in my virtual environments (and I use new ones pretty regularly).

lukpueh commented 7 years ago

@vladimir-v-diaz: I don't know what happened in your development environment but if you installed canonicaljson (which requires simplejson and frozendict, cf. your error messages) using the usual tools (i.e. pip) you would have those packages installed automatically as dependencies (cf. canonicaljson's setup.py).

Moreover, uptane lists canonicaljson in its setup.py and in its dev-requirements.txt so the requirement chain should work.

I think we can close this as a non-issue.

If we really want to catch the edge case where someone does not install uptane correctly and therefor misses dependencies, we would have to override the import statements in canonicaljson. I think that's not worth the effort.

awwad commented 7 years ago

I think Vlad is probably suggesting that we flatten the dependencies. If simplejson is a dependency (and it kinda is, it looks like), and if it is possible to not have it (that's the part I'm somewhat more confused about), then I guess it should be in dev-requirements.txt. No, @vladimir-v-diaz?

vladimir-v-diaz commented 7 years ago

It might just be a corrupted installation. I will try to reproduce the issue with the latest version of pip and setuptools, and if everything works as expected we can close this issue.

However, it seems safer to just list all the expected dependencies and their versions.

lukpueh commented 7 years ago

Let's wrap up our (@vladimir-v-diaz, @awwad and I) discussion about pro and contra of flattening the requirements file, i.e. explicitly listing dependencies and sub-dependencies with explicit version numbers.

Pros:

Contras:

lukpueh commented 7 years ago

I vote for closing this as non-issue.

vladimir-v-diaz commented 7 years ago

I think these articles on package management with pip and pip-tools are helpful:

https://www.kennethreitz.org/essays/a-better-pip-workflow http://nvie.com/posts/better-package-management/ http://nvie.com/posts/pin-your-packages/

The general idea is to have two pip requirements files (requirements.txt and requirements-to-freeze.txt) to allow deterministic and production builds, respectively. The pip-tools library is used to make sure your project's dependencies are up to date.

awwad commented 7 years ago

Food for thought, but while I'm conflicted about flattening, this isn't something that should generally be encountered, and I think we can probably close for now until it proves to be a problem in the future.