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

pythonPackages.sqlalchemy: 1.3.23 -> 1.4.7 #119232

Closed
wants to merge 1 commit into from
Closed

pythonPackages.sqlalchemy: 1.3.23 -> 1.4.7 #119232

wants to merge 1 commit into from

Conversation

vanzef
Copy link
Contributor

@vanzef vanzef commented Apr 12, 2021

Motivation for this change

Update since 1.3.* is in maintenance mode

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@SuperSandro2000
Copy link
Member

/rebase staging

SuperSandro2000
SuperSandro2000 reviewed Apr 12, 2021
View reviewed changes
pkgs/development/python-modules/sqlalchemy/default.nix
version = "1.3.23";
version = "1.4.7";

disabled = !(isPy27 || pythonAtLeast "3.6");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
disabled = !(isPy27 || pythonAtLeast "3.6");
disabled = pythonOlder "3.6";

Copy link
Member

Choose a reason for hiding this comment

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

python-2.7 is still supported by sqlalchemy-1.4. Only versions before 3.6 in the 3 series are not (i.e between 2.7 excluded to 3.5 included).

Considering we do not ship any python3 release before 3.6 in nixpkgs, it looks like this all line could be dropped altogether. I admit having the requirement explicitly stated are good, and do not mind if they are kept.

SuperSandro2000
SuperSandro2000 reviewed Apr 12, 2021
View reviewed changes
pkgs/development/python-modules/sqlalchemy/default.nix Outdated
@@ -30,14 +25,16 @@ buildPythonPackage rec {

pytestFlagsArray = [ "-n auto" ];

buildInputs = lib.optional isPy3k greenlet;
Copy link
Member

Choose a reason for hiding this comment

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

Isn't greenlet required at runtime?

SuperSandro2000
SuperSandro2000 reviewed Apr 12, 2021
View reviewed changes
pkgs/development/python-modules/sqlalchemy/default.nix
postInstall = ''
sed -e 's:--max-worker-restart=5::g' -i setup.cfg
'';

dontUseSetuptoolsCheck = true;

# disable mem-usage tests on mac, has trouble serializing pickle files
disabledTests = lib.optionals isPy35 [ "exception_persistent_flush_py3k "]
disabledTests = lib.optionals isPy3k [ "exception_persistent_flush_py3k "]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
disabledTests = lib.optionals isPy3k [ "exception_persistent_flush_py3k "]
disabledTests = [ "exception_persistent_flush_py3k "]

@github-actions github-actions bot changed the base branch from master to staging April 12, 2021 15:27
@github-actions github-actions bot closed this Apr 12, 2021
@github-actions
Copy link
Contributor

Rebased, please reopen the pull request to restart CI

@SuperSandro2000
Copy link
Member

@vanzef can you resolve the merge conflict?

@r-rmcgibbo
Copy link

r-rmcgibbo commented Apr 12, 2021 •
edited

Result of nixpkgs-review pr 119232 at c1e48960 run on aarch64-linux 1

148 packages marked as broken and skipped:
  • bareos
  • belle-sip
  • bonfire
  • cassandra_2_1
  • cassandra_2_2
  • couchdb
  • cvc4
  • expliot
  • gnuradio3_7
  • gnuradio3_7Packages.ais
  • ...
111 packages failed to build:
970 packages skipped due to time constraints:
  • DisnixWebService
  • R
  • abcl
  • adapta-gtk-theme
  • adoptopenjdk-icedtea-web
  • aiodnsbrute
  • alloy (alloy5)
  • alloy4
  • ansible (ansible_2_10)
  • ansible-lint (python38Packages.ansible-lint)
  • ...
34 packages built successfully:
  • acd-cli
  • anki
  • ankisyncd
  • python38Packages.aiocontextvars
  • python38Packages.celery
  • python38Packages.kombu
  • python38Packages.mautrix (python38Packages.mautrix-appservice)
  • python38Packages.pymodbus
  • python38Packages.sqlalchemy
  • python39Packages.aiocontextvars
  • python39Packages.aioinflux
  • python39Packages.apptools
  • python39Packages.bkcharts
  • python39Packages.csvs-to-sqlite
  • python39Packages.drms
  • python39Packages.flammkuchen
  • python39Packages.influxdb
  • python39Packages.kombu
  • python39Packages.mautrix (python39Packages.mautrix-appservice)
  • python39Packages.mplfinance
  • python39Packages.pandas
  • python39Packages.pandas-datareader
  • python39Packages.partd
  • python39Packages.ppscore
  • python39Packages.pyezviz
  • python39Packages.pytrends
  • python39Packages.rapidfuzz
  • python39Packages.seaborn
  • python39Packages.sqlalchemy
  • python39Packages.vega
  • python39Packages.vega_datasets
  • python39Packages.vidstab
  • python39Packages.yfinance
  • ytcc
1 suggestion:
  • warning: maintainers-missing

    Package does not have a maintainer. Consider adding yourself?

    Near pkgs/development/python-modules/sqlalchemy/default.nix:40:3:

       |
40 |   meta = with lib; {
   |   ^

Note that build failures may predate this PR, and could be nondeterministic or hardware dependent.
Please exercise your independent judgement.

(Note from @rmcgibbo: this build was run before the PR was re-targetted to staging)

@vanzef
Copy link
Contributor Author

vanzef commented Apr 12, 2021

@sternenseemann I did as you suggested, but didn't get any conflicts and it seems that everything is back to mess.

P.S. Is there a way to fix all this without pinging everyone each time?

@sternenseemann
Copy link
Member

sternenseemann commented Apr 12, 2021 •
edited

I've went ahead and did that for you. Apologies, I forgot to mention that you need to do an interactive rebase and drop every commit except the one you are trying to get merged (not sure if there is a more convenient way for doing this).

drewrisinger
drewrisinger reviewed Apr 12, 2021
View reviewed changes
pkgs/development/python-modules/sqlalchemy/default.nix
@@ -30,14 +27,17 @@ buildPythonPackage rec {

pytestFlagsArray = [ "-n auto" ];

propagatedBuildInputs = lib.optional isPy3k greenlet
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
propagatedBuildInputs = lib.optional isPy3k greenlet
propagatedBuildInputs = [ greenlet ]

Only py3.X is supported (by disable), so the optional is not needed.

@SuperSandro2000
Copy link
Member

@sternenseemann I did as you suggested, but didn't get any conflicts and it seems that everything is back to mess.

P.S. Is there a way to fix all this without pinging everyone each time?

Using the rebase action I used and you reverted :)

@dotlambda
Copy link
Member

superseded by #121854

@dotlambda dotlambda closed this Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lsix lsix lsix left review comments

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@drewrisinger drewrisinger drewrisinger left review comments

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

Assignees
No one assigned
Labels
6.topic: python 10.rebuild-darwin: 501+ 10.rebuild-darwin: 1001-2500 10.rebuild-linux: 501+ 10.rebuild-linux: 1001-2500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@vanzef @SuperSandro2000 @r-rmcgibbo @sternenseemann @dotlambda @lsix @drewrisinger