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

Set maximum version for fsspec because it is super unstable #59

Merged
merged 2 commits into from
Mar 27, 2024

Conversation

danielhfrank
Copy link
Contributor

@danielhfrank danielhfrank commented Mar 27, 2024

  • When working on Update CI and switch to python 3.11 #57, I had to fix some behavior changes that crept in via an fsspec upgrade. In order to catch these issues in the future, I also set up a weekly test to catch any more regressions like that, which are easily possible since we don't pin dependencies in this library. Well, it's been two whole weeks, and fsspec released an updated version that somehow changed behavior again and showed up in the weekly test.
  • I didn't really want to spend time chasing down what exactly changed, but I can see by toggling versions and running tests that something did. This is a silly problem to chase, so I put a version restriction that effectively keeps articat on fsspec==2024.2.0, which is what facets uses.

@danielhfrank danielhfrank marked this pull request as ready for review March 27, 2024 00:30
Copy link
Member

@dhimmel dhimmel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wild. Are we using fsspec is a bespoke way or are they making frequent breaking changes to common functionality? (If this question takes more than 3 minutes to answer, please ignore 😄 ).

@@ -1,5 +1,5 @@
fire >= 0.4
fsspec >= 2021.7.0
fsspec >= 2021.7.0, < 2024.3.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 slight preference to comment with a link to this PR to save future editors a blame lookup.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 892ad57

@danielhfrank
Copy link
Contributor Author

At least the last time that this came up, the functionality that was changing was around an fsspec implementation of glob, in a way that was attempting to work cross-platform. We make frequent use of the ** glob pattern, which is a little unusual I guess.

It would definitely have taken me more than 3 minutes to find out if that was in fact the problem again here, so I just pinned the version 😉

@danielhfrank danielhfrank merged commit 7c869ce into main Mar 27, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants