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

Pin SQAlchemy to <1.4.0 #11629

Merged
merged 3 commits into from
Mar 16, 2021
Merged

Pin SQAlchemy to <1.4.0 #11629

merged 3 commits into from
Mar 16, 2021

Conversation

jdavcs
Copy link
Member

@jdavcs jdavcs commented Mar 16, 2021

1.4 released; our codebase not compatible yet.

What did you do?

Pin SQAlchemy to <1.4.0

Why did you make this change?

SQLAlchemy 1.4 has been released. Galaxy's code is not fully compatible yet.
Pin is temporary; will be removed when code is made compatible.

How to test the changes?

  • Instructions for manual testing are as follows:
    make update-dependencies
    Check that sqlalchemy is >= 1.3.22 < 1.4 in requirements.txt

@jdavcs jdavcs added this to the 21.05 milestone Mar 16, 2021
pyproject.toml Outdated Show resolved Hide resolved
@nsoranzo nsoranzo mentioned this pull request Mar 16, 2021
3 tasks
@nsoranzo
Copy link
Member

Can you please also add ~=1.3.22 to the SQLAlchemy requirements in packages/data/requirements.txt , packages/web_framework/requirements.txt , packages/web_stack/requirements.txt ?

@jmchilton
Copy link
Member

jmchilton commented Mar 16, 2021

I think in the packages it should be ~=1.3. The library code shouldn't be overly constrained the way the application is. I would expect that galaxy-data is going to work fine in an application that has a 1.3.21 pinned sql alchemy dependency for instance, and the dependencies should reflect.

xref https://www.python.org/dev/peps/pep-0440/#compatible-release for other unfamiliar with the syntax.

Update: Maybe I'm reading that wrong, excuse me if I'm not making sense.

Update 2: Is ~=1.3.0 what I actually am trying to say? Is that [1.3,1.4) mathematically 😆 - the true and correct syntax.

@jdavcs
Copy link
Member Author

jdavcs commented Mar 16, 2021

I think in the packages it should be ~=1.3. The library code shouldn't be overly constrained the way the application is. I would expect that galaxy-data is going to work fine in an application that has a 1.3.21 pinned sql alchemy dependency for instance, and the dependencies should reflect.
xref https://www.python.org/dev/peps/pep-0440/#compatible-release for other unfamiliar with the syntax.
Update: Maybe I'm reading that wrong, excuse me if I'm not making sense.
Update 2: Is ~=1.3.0 what I actually am trying to say? Is that [1.3,1.4) mathematically - the true and correct syntax.

I think Poetry does not fully support PEP-440. Here are the dependency specifiers it uses. There are multiple issues discussing compatibility with PEP-440, here's the most relevant, I think. So I don't think we can use the ~= compatibility specifier. We can do the same with this, I think:
~1.3.22 will result in >=1.3.22, <1.4.0
For the library, then, it could be ~1.3 which would expand the range to >=1.3.0, <1.4.0
@jmchilton does this look right?

EDIT: but poetry doesn't touch packages.. Sorry I missed this. Should I use ~1.3 (or ~1.3.0 - I'm checking this now) after all?

@nsoranzo
Copy link
Member

We can directly use >=1.3.0, <1.4.0 in both pyproject.toml and the packages' requirements.txt , this syntax works for both poetry and pip.

@jdavcs
Copy link
Member Author

jdavcs commented Mar 16, 2021

I used >=1.3.0, <1.4.0 for packages and >=1.3.22, <1.4.0 for the galaxy app.

jdavcs and others added 3 commits March 16, 2021 13:55
1.4 released; our codebase not compatible yet.
Co-authored-by: Nicola Soranzo <nicola.soranzo@gmail.com>
@jmchilton
Copy link
Member

Amazing, thanks so much!

@jdavcs
Copy link
Member Author

jdavcs commented Mar 16, 2021

Thank you for reviewing!

@jdavcs jdavcs merged commit b0173a3 into galaxyproject:dev Mar 16, 2021
@github-actions
Copy link

This PR was merged without a "kind/" label, please correct.

@jdavcs jdavcs mentioned this pull request Mar 16, 2021
4 tasks
@nsoranzo nsoranzo changed the title Pin SQAlchemy to ~1.3.22 Pin SQAlchemy to <1.4.0 Mar 17, 2021
@jdavcs jdavcs deleted the dev_sa_pin branch April 5, 2021 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants