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.

Sign up for GitHub

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

Jump to bottom

Pin botocore version #5184

Merged
merged 3 commits into from Jun 19, 2018
Merged

Pin botocore version #5184

merged 3 commits into from Jun 19, 2018

Conversation

ledor473
Copy link
Contributor

@ledor473 ledor473 commented Jun 12, 2018 •
edited

Fixes for #5175

I wasn't sure if I had to change it in setup.py as well... Mostly because the recent changes for Kombu were only in requirements.txt and that setup.py has a different boto3 version

@codecov-io
Copy link

codecov-io commented Jun 12, 2018 •
edited

Codecov Report

Merging #5184 into master will increase coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5184      +/-   ##
==========================================
+ Coverage   63.45%   63.55%   +0.09%     
==========================================
  Files         261      261              
  Lines       19842    20112     +270     
  Branches     1998     1998              
==========================================
+ Hits        12591    12782     +191     
- Misses       7242     7321      +79     
  Partials        9        9
Impacted Files Coverage Δ
superset/models/core.py 87.64% <0%> (+1.09%) ⬆️
superset/db_engine_specs.py 56.23% <0%> (+2.51%) ⬆️

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 ccf2110...1801e1e. Read the comment docs.

john-bodley
john-bodley reviewed Jun 13, 2018
View reviewed changes
requirements.txt Outdated
@@ -1,5 +1,6 @@
bleach==2.1.2
boto3==1.4.7
botocore<1.8.0,>=1.7.0
Copy link
Member

Choose a reason for hiding this comment

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

You should pin to a specific version here.

@ledor473
Copy link
Contributor Author

Running pip3 freeze gave me the version 1.7.48. I've updated the PR with that version

@mistercrunch
Copy link
Member

If you know a certain version is not working with Superset for sure, we should add a range in setup.py as well. So say if there are known issues with 1.8.0 I would add <1.8.0 in setup.py as well.

@ledor473
Copy link
Contributor Author

@mistercrunch should I put the same boto3 version in setup.py? Just so there's no discrepancy between the two?

@mistercrunch
Copy link
Member

requirements.txt should be pinned as much as possible, setup.py should be unpinned, but define version ranges when we know that >={some version} isn't compatible for sure.

@ledor473
Copy link
Contributor Author

Sounds good! I've updated the PR

@mistercrunch mistercrunch merged commit 70679d4 into apache:master Jun 19, 2018
john-bodley added a commit that referenced this pull request Jun 20, 2018
@john-bodley
Revert "Pin botocore version (#5184)"
f2fd1f6 
This reverts commit 70679d4.
john-bodley added a commit that referenced this pull request Jun 20, 2018
@john-bodley
Revert "[perf] add webpack 4 + SplitChunks + lazy load visualizations" (
62427c8 
#5253)

* Revert "[sqllab] Fix sql lab resolution link (#5216)"

This reverts commit 93cdf60.

* Revert "Pin botocore version (#5184)"

This reverts commit 70679d4.

* Revert "Describe the use of custom OAuth2 authorization servers (#5220)"

This reverts commit a84f430.

* Revert "[bubble-chart] Fixing issue w/ metric names (#5237)"

This reverts commit 5c106b9.

* Revert "[adhoc-filters] Adding adhoc-filters to all viz types (#5206)"

This reverts commit d483ed1.

* Revert "[perf] add webpack 4 + SplitChunks + lazy load visualizations (#5240)"

This reverts commit 1fc4ee0.
john-bodley added a commit to john-bodley/superset that referenced this pull request Jun 20, 2018
@john-bodley
Revert "[perf] add webpack 4 + SplitChunks + lazy load visualizations" (
5d8c317 
apache#5253)

* Revert "[sqllab] Fix sql lab resolution link (apache#5216)"

This reverts commit 93cdf60.

* Revert "Pin botocore version (apache#5184)"

This reverts commit 70679d4.

* Revert "Describe the use of custom OAuth2 authorization servers (apache#5220)"

This reverts commit a84f430.

* Revert "[bubble-chart] Fixing issue w/ metric names (apache#5237)"

This reverts commit 5c106b9.

* Revert "[adhoc-filters] Adding adhoc-filters to all viz types (apache#5206)"

This reverts commit d483ed1.

* Revert "[perf] add webpack 4 + SplitChunks + lazy load visualizations (apache#5240)"

This reverts commit 1fc4ee0.

(cherry picked from commit 62427c8)
@jeffreythewang
Copy link
Contributor

jeffreythewang commented Jun 26, 2018 •
edited

It seems like boto3 verisions 1.4.6 and 1.4.8 are both incompatible with botocore>=1.7.0, <1.8.0, and only 1.4.7 satisfies the requirement. Does it make sense to pin boto3 to 1.4.7 in setup.py?

@mistercrunch
Copy link
Member

@jeffreythewang if this is the only version that work then yes for sure

Now I'm thinking that ideally Superset would not have any dependencies on boto, only extra_requires, but let's just get this working first. I think I hit this issue last week but had no time to investigate...

@jeffreythewang jeffreythewang mentioned this pull request Jun 26, 2018
Merged
timifasubaa pushed a commit to airbnb/superset-fork that referenced this pull request Jul 25, 2018
@ledor473 @timifasubaa
Pin botocore version (apache#5184)
c129e74 
* Pin botocore version

* Version range for botocore
timifasubaa pushed a commit to airbnb/superset-fork that referenced this pull request Jul 25, 2018
@john-bodley @timifasubaa
Revert "[perf] add webpack 4 + SplitChunks + lazy load visualizations" (
354cb85 
apache#5253)

* Revert "[sqllab] Fix sql lab resolution link (apache#5216)"

This reverts commit 93cdf60.

* Revert "Pin botocore version (apache#5184)"

This reverts commit 70679d4.

* Revert "Describe the use of custom OAuth2 authorization servers (apache#5220)"

This reverts commit a84f430.

* Revert "[bubble-chart] Fixing issue w/ metric names (apache#5237)"

This reverts commit 5c106b9.

* Revert "[adhoc-filters] Adding adhoc-filters to all viz types (apache#5206)"

This reverts commit d483ed1.

* Revert "[perf] add webpack 4 + SplitChunks + lazy load visualizations (apache#5240)"

This reverts commit 1fc4ee0.
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
@ledor473 @mistercrunch
Pin botocore version (apache#5184)
261972c 
* Pin botocore version

* Version range for botocore
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
@john-bodley
Revert "[perf] add webpack 4 + SplitChunks + lazy load visualizations" (
8d9678c 
apache#5253)

* Revert "[sqllab] Fix sql lab resolution link (apache#5216)"

This reverts commit 93cdf60.

* Revert "Pin botocore version (apache#5184)"

This reverts commit 70679d4.

* Revert "Describe the use of custom OAuth2 authorization servers (apache#5220)"

This reverts commit a84f430.

* Revert "[bubble-chart] Fixing issue w/ metric names (apache#5237)"

This reverts commit 5c106b9.

* Revert "[adhoc-filters] Adding adhoc-filters to all viz types (apache#5206)"

This reverts commit d483ed1.

* Revert "[perf] add webpack 4 + SplitChunks + lazy load visualizations (apache#5240)"

This reverts commit 1fc4ee0.
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.26.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@john-bodley john-bodley john-bodley approved these changes

Assignees
No one assigned
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.26.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ledor473 @codecov-io @mistercrunch @jeffreythewang @john-bodley