Skip to content

chore(deps): set correct version of flask_caching#21228

Closed
KNOEEE wants to merge 1 commit intoapache:masterfrom
KNOEEE:refine-version
Closed

chore(deps): set correct version of flask_caching#21228
KNOEEE wants to merge 1 commit intoapache:masterfrom
KNOEEE:refine-version

Conversation

@KNOEEE
Copy link

@KNOEEE KNOEEE commented Aug 29, 2022

SUMMARY

We need to set the highest version of flask-caching lower than v2.0.0, because it need "cachelib >= 0.9.0" since v2.0 which is not compatible with requirements of superset("cachelib>=0.4.1,<0.5").
Ref: https://github.com/pallets-eco/flask-caching/blob/v2.0.0/setup.py

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

It failed down if building with flask-caching2.0.0+.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@KNOEEE KNOEEE changed the title fix: set correct version of flask_caching chore(deps): set correct version of flask_caching Aug 29, 2022
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️
We hope to see you in our Slack community too!

@villebro
Copy link
Member

@KNOEEE did you check if it's possible to bump cachelib to the latest version? I believe the <0.5 restriction for cachelib is mainly there because 0.4 was the most recent version at the time that the lib was last bumped, and because of semver 0.5 is not guaranteed not to introduce a breaking change over 0.4.

@codecov
Copy link

codecov bot commented Aug 29, 2022

Codecov Report

Merging #21228 (e3066ac) into master (5f76ac9) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #21228   +/-   ##
=======================================
  Coverage   66.40%   66.40%           
=======================================
  Files        1783     1783           
  Lines       68087    68087           
  Branches     7261     7261           
=======================================
  Hits        45215    45215           
  Misses      21007    21007           
  Partials     1865     1865           
Flag Coverage Δ
hive 53.11% <ø> (ø)
mysql 81.00% <ø> (ø)
postgres 81.05% <ø> (ø)
presto 53.01% <ø> (ø)
python 81.53% <ø> (ø)
sqlite 79.57% <ø> (ø)
unit 50.67% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@KNOEEE
Copy link
Author

KNOEEE commented Aug 29, 2022

@KNOEEE did you check if it's possible to bump cachelib to the latest version? I believe the <0.5 restriction for cachelib is mainly there because 0.4 was the most recent version at the time that the lib was last bumped, and because of semver 0.5 is not guaranteed not to introduce a breaking change over 0.4.

No, I didn't. Maybe I can try it. If it's all good, we should set deps as cachelib>=0.9.0,<0.10.0 and flask-caching>=1.10.0, right?

@villebro
Copy link
Member

I'd prefer to bump the full chain of deps if it's possible, i.e. Flask-Caching to >=2.0.1, <3 and then see what other deps need to be bumped. I'm almost wondering if we can't remove the explicit cachelib requirement, as IMO Flask-Caching is our main dep for this functionality.

@EugeneTorap EugeneTorap mentioned this pull request Feb 6, 2023
9 tasks
@rusackas
Copy link
Member

rusackas commented Feb 2, 2024

Closing this since it seems like we're on "flask-caching>=2.1.0, <3", now.

@rusackas rusackas closed this Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants