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

chore: Bump pip-compile-multi #14633

Merged

Conversation

john-bodley
Copy link
Member

@john-bodley john-bodley commented May 14, 2021

SUMMARY

Rumor has it that the pip-compile-multi ecosystem was likely borked, probably due to changes which i) used an improper version of pip-compile-multi/pip-tools, or ii) updates to setup.py etc. which didn't update the pinned requirements.

This PR updates pip-compile-multi and updates the requirements/*.txt files without upgrading any of the packages, via:

pip-compile-multi --only-name integration --upgrade-package pip-compile-multi 
python3.8 -m pip install -r requirements/integration.txt 
pip-compile-multi --no-upgrade

Note we likely should look into something like http://dependabot.com/ or similar for ensuring dependencies remain fresh. See the Slack thread in the #committers channel here for more detail.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

  • Has associated issue:
  • 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

@john-bodley john-bodley force-pushed the john-bodley--bump-pip-compile-multi branch from 6ce8a91 to 3db127a Compare May 14, 2021 04:11
@codecov
Copy link

codecov bot commented May 14, 2021

Codecov Report

Merging #14633 (3db127a) into master (2bd0b62) will increase coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14633      +/-   ##
==========================================
+ Coverage   77.39%   77.47%   +0.08%     
==========================================
  Files         958      958              
  Lines       48486    48486              
  Branches     5679     5679              
==========================================
+ Hits        37524    37563      +39     
+ Misses      10762    10723      -39     
  Partials      200      200              
Flag Coverage Δ
hive 80.96% <ø> (ø)
mysql 81.24% <ø> (ø)
postgres 81.26% <ø> (ø)
presto 80.95% <ø> (?)
python 81.78% <ø> (+0.15%) ⬆️
sqlite 80.88% <ø> (ø)

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

Impacted Files Coverage Δ
superset/models/core.py 89.13% <0.00%> (+0.27%) ⬆️
superset/connectors/sqla/models.py 90.26% <0.00%> (+1.44%) ⬆️
superset/db_engine_specs/presto.py 89.89% <0.00%> (+5.47%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2bd0b62...3db127a. Read the comment docs.

@john-bodley john-bodley requested a review from nytai May 14, 2021 06:32
Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM - I'd really like to look into the dependabot option to replace pip-compile-multi. If help is needed, I can try to find time to work on that.

@john-bodley
Copy link
Member Author

john-bodley commented May 14, 2021

BTW I'm a fan of pip-compile-multi as it groups dependencies by roles in a maintainable way. Additionally per the Slack thread it's important that we also update the pre-commit hooks (ideally at the same time) and I'm unsure if dependabot handles this.

@villebro if you or @nytai have time to explore this that would be great. I think one learning of auto-updating dependencies on a regular basis for a number of internal projects is it really only works if one has good test coverage, linting etc. which boosts one's confidence of the legitimacy of the PR, i.e., increases the likelihood of merging with little to no manual testing. The general lack of pylint coverage could be problematic.

@john-bodley john-bodley merged commit c55418d into apache:master May 14, 2021
@john-bodley john-bodley deleted the john-bodley--bump-pip-compile-multi branch May 14, 2021 17:06
cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
Co-authored-by: John Bodley <john.bodley@airbnb.com>
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
Co-authored-by: John Bodley <john.bodley@airbnb.com>
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
Co-authored-by: John Bodley <john.bodley@airbnb.com>
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XXL 🚢 1.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants