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

fix: versions aren't pinned and pip-compile-validate doesn't detect dependency changes #15573

Closed
wants to merge 5 commits into from

Conversation

ofekisr
Copy link
Contributor

@ofekisr ofekisr commented Jul 7, 2021

SUMMARY

A better best practice is to put all project dependencies into the requirements file instead of setup.py
so adding a new dependency will Imply running pip-compile-multi.
When adding a new dependency into setup.py, pip-comiple-multi verify will not detect the change.
this PR try to prevent issues like #15527

Actually, I adapted the way pip-compile-multi did
and you can take another reference here: pip-compile-issue

another Important issue this PR fix is the packages that were in setup.py weren't pinned versions so the client who installs superset is not guaranteed the superset's dependencies will be the same when code was merged

note - before the commit, after moving the packages from setup.py I ran pip-compile-multi --no-upgrade to ensure
that all requirements files are ok

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Before opening the pr, I ran tox to ensure superset init is running to prove superset was installed ( successful running the ci inside tox)
you can try run tox by yourself
in addition, if the pr checks success, it mean nothing breaks

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

@amitmiran137 amitmiran137 requested a review from a team July 7, 2021 15:23
@codecov
Copy link

codecov bot commented Jul 7, 2021

Codecov Report

Merging #15573 (0a5c369) into master (9ed8ce5) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #15573   +/-   ##
=======================================
  Coverage   76.95%   76.95%           
=======================================
  Files         976      976           
  Lines       51326    51326           
  Branches     6912     6912           
=======================================
+ Hits        39498    39500    +2     
+ Misses      11607    11605    -2     
  Partials      221      221           
Flag Coverage Δ
hive 81.30% <ø> (+<0.01%) ⬆️
javascript 71.40% <ø> (ø)
mysql 81.56% <ø> (+<0.01%) ⬆️
postgres 81.59% <ø> (+<0.01%) ⬆️
presto 81.29% <ø> (+<0.01%) ⬆️
python 82.12% <ø> (+<0.01%) ⬆️
sqlite 81.19% <ø> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
superset/config.py 91.83% <0.00%> (+0.68%) ⬆️

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 9ed8ce5...0a5c369. Read the comment docs.

@ofekisr ofekisr reopened this Jul 8, 2021
@pull-request-size pull-request-size bot added size/L and removed size/XS labels Jul 8, 2021
@ofekisr ofekisr changed the title fix: pip-compile-validate does not detect dependency changes fix: versions doesn't pinned and pip-compile-validate doesn't detect dependency changes Jul 8, 2021
@ofekisr ofekisr changed the title fix: versions doesn't pinned and pip-compile-validate doesn't detect dependency changes fix: versions don't pinned and pip-compile-validate doesn't detect dependency changes Jul 8, 2021
@ofekisr ofekisr changed the title fix: versions don't pinned and pip-compile-validate doesn't detect dependency changes fix: versions aren't pinned and pip-compile-validate doesn't detect dependency changes Jul 8, 2021
@ofekisr ofekisr force-pushed the fix/use_pip_compile_better branch from 3871531 to 9d96d8b Compare July 11, 2021 07:30
# Conflicts:
#	requirements/base.txt
#	requirements/integration.txt
#	requirements/testing.txt
@john-bodley
Copy link
Member

john-bodley commented Jul 13, 2021

@ofekisr I don't believe this is the right approach, especially as we should strive to always ensure that all the dependencies are pinned. In #15574 I've added instructions regarding how and when users should recreate the dependencies.

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.

None yet

3 participants