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: remove fixed psycopg2 version #27907

Closed
wants to merge 3 commits into from
Closed

chore: remove fixed psycopg2 version #27907

wants to merge 3 commits into from

Conversation

raphaelauv
Copy link

@raphaelauv raphaelauv commented Apr 4, 2024

SUMMARY

bump psycopg2

allow upper versions of psycopg2

@github-actions github-actions bot added the doc Namespace | Anything related to documentation label Apr 4, 2024
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! Not signed up? Use our Slack App to self-register.

@@ -155,7 +155,7 @@ connect to those datasources in your Superset installation:
```yaml
bootstrapScript: |
#!/bin/bash
pip install psycopg2==2.9.6 \
pip install psycopg2==2.9.9 \
Copy link
Member

@mistercrunch mistercrunch Apr 4, 2024

Choose a reason for hiding this comment

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

this should probably reference the optional dependencies as inpip install -e .[postgres], which would make it consistent and in sync with the ranges defined in pyproject.toml

pyproject.toml Outdated
@@ -150,7 +150,7 @@ ocient = [
oracle = ["cx-Oracle>8.0.0, <8.1"]
pinot = ["pinotdb>=0.3.3, <0.4"]
playwright = ["playwright>=1.37.0, <2"]
postgres = ["psycopg2-binary==2.9.6"]
postgres = ["psycopg2-binary==2.9.9"]
Copy link
Member

Choose a reason for hiding this comment

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

Personally I'd vote for leaving this open as in psycopg2-binary>=2.9.6

Copy link
Author

Choose a reason for hiding this comment

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

what about

psycopg2-binary>=2.9.6,<2.10 ?

since postgres is the most famous backend for superset itself

and also cause superset do not associate constraint files with release

so I don't want to be the guy that make fail future installation in case of breaking change on psycopg2 ( that is very unlikely cause psycopg3 is already out, but we never know )

Copy link
Member

Choose a reason for hiding this comment

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

Requirements are ideally intervals rather than specific versions—which will get pinned for CI et al. when you run pip-compile-multi. The concern with pinning a version here is it could severely reduce the feasibly space when pip-compile-multi tries to freeze all packages.

Copy link
Author

Choose a reason for hiding this comment

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

I don't understand your message @john-bodley , what are you talking about ?

@mistercrunch
Copy link
Member

Curious on the motives for the bump, just common hygiene or is there an issue / bug with 2.9.6?

Generally I like to have code comments when we put an upper bound or a pin on a version so that we know what was keeping us from upgrading when the bound was set.

@raphaelauv
Copy link
Author

hey , thanks for the review

2.9.9 support python 3.12 , so it's a first step on superset 3.12 support

@mistercrunch
Copy link
Member

About python support, I wrote this recently, kind of a request for comment around the topic #27906

@john-bodley
Copy link
Member

@raphaelauv could you please complete the PR description? This helps to provide the necessary context to the reviewers.

@raphaelauv raphaelauv changed the title chore: psycopg2 2.9.9 chore: remove fixed psycopg2 version Apr 8, 2024
@raphaelauv raphaelauv closed this Jul 4, 2024
@raphaelauv raphaelauv deleted the bump/psycopg2 branch July 4, 2024 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Namespace | Anything related to documentation size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants