Upgrade to python 3, flask 2, and other dependencies - #84
Conversation
This comment has been minimized.
This comment has been minimized.
| requests>=2.23.0 | ||
| SQLAlchemy>=1.3.16 | ||
| requests>=2.27.1 | ||
| SQLAlchemy>=1.3.16,<2.0 |
There was a problem hiding this comment.
big API changes in sqlalchemy2, so I decided to pin this here for now until we can be sure that mbdata works with it
58a7032 to
20ec67c
Compare
| use_scm_version=True, | ||
| setup_requires=['setuptools_scm'], | ||
| install_requires=open("requirements.txt").read().split(), | ||
| install_requires=open("requirements.txt").read().splitlines(), |
There was a problem hiding this comment.
this fixes some of the issues that we had in requirements file (it was splitting on whitespace, not newline). We could also consider removing comments:
[line for line in open("requirements.txt").read().splitlines() if line and not line.startswith('#')]
Still doesn't remove inline comments. I wonder if a specific requirements parser would be a better idea?
There was a problem hiding this comment.
This fix with a comment in requirements.txt is good enough IMO.
That said, I guess its fine to use a special parser for this or moving this other way around as that answer suggests etc. Maybe open a ticket for later improvement.
There was a problem hiding this comment.
of course, perhaps the better solution would be to switch to poetry/pyproject.toml instead, which should be able to reuse the same dependency list...
This comment has been minimized.
This comment has been minimized.
|
I don't know why that guard exists but if I were to guess, redis-4.0 (redis 4 wasn't released until much later that year but the documentation of redis 3.5.3 specified that 4.0 wouldn't support py2.7) does not support py2.7 but at time of #45 BU still supported py2.7. |
amCap1712
left a comment
amCap1712
left a comment
There was a problem hiding this comment.
LGTM, Thanks!
But we should probably test at least 1 downstream app before merging and releasing.
20ec67c to
bcc913c
Compare
This comment has been minimized.
This comment has been minimized.
2 participants
I'm not sure why redis had a guard on it to not install versions greater than v4. @amCap1712 do you know why? I didn't add a specific reason in #45