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

Jump to bottom

python3Packages.{websockets, uvicorn,python-engineio}: bump an… #68939

Merged
merged 3 commits into from Sep 20, 2019
Merged

python3Packages.{websockets, uvicorn,python-engineio}: bump an… #68939

merged 3 commits into from Sep 20, 2019

Conversation

jonringer
Copy link
Contributor

@jonringer jonringer commented Sep 16, 2019 •
edited

Motivation for this change

#68361

bump a few related packages, they were all broken before. wanted nix-review to pass.

enabled tests on uvicorn and websockets to make sure I wasn't breaking anything by disregarding version bounds.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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.
Notify maintainers

cc @

https://github.com/NixOS/nixpkgs/pull/68939
12 package were build:
datasette gdbgui grab-site python37Packages.fastapi python37Packages.flask-socketio python37Packages.hbmqtt python37Packages.python-engineio python37Packages.python-socketio python37Packages.sanic python37Packages.starlette python37Packages.uvicorn python37Packages.websockets

@jonringer jonringer changed the title Bump websockets python3Packages.{websockets, uvicorn,python-engineio}: bump and fix builds Sep 16, 2019
@ofborg ofborg bot requested review from wd15 and Mic92 September 16, 2019 20:43
B4dM4n
B4dM4n reviewed Sep 16, 2019
View reviewed changes
Copy link
Contributor

@B4dM4n B4dM4n left a comment

Choose a reason for hiding this comment

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

The updated versions seem fine, but I put some comments inline.

I'm also not a fan of reordering lines, just for the sake of them being alphabetically sorted (like in python-engineio/default.nix). It makes it harder to reason about the actual changes. But this is just my personal opinion.

pkgs/development/python-modules/pgadmin4/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/uvicorn/default.nix Show resolved Hide resolved
pkgs/development/python-modules/uvicorn/default.nix Outdated Show resolved Hide resolved
@wd15
Copy link
Contributor

wd15 commented Sep 17, 2019

Agree regarding reordering, but leave it to your discretion.

Also, is Nixpkgs moving to fetchFromGitHub rather than fetchPypi in general?

Result of nix-review 1 on Ubuntu using x86_64.

12 packages were built:
  • datasette (python37Packages.datasette)
  • gdbgui (python37Packages.datasette)
  • grab-site (python37Packages.datasette)
  • python37Packages.fastapi (python37Packages.datasette)
  • python37Packages.flask-socketio (python37Packages.datasette)
  • python37Packages.hbmqtt (python37Packages.datasette)
  • python37Packages.python-engineio (python37Packages.datasette)
  • python37Packages.python-socketio (python37Packages.datasette)
  • python37Packages.sanic (python37Packages.datasette)
  • python37Packages.starlette (python37Packages.datasette)
  • python37Packages.uvicorn (python37Packages.datasette)
  • python37Packages.websockets (python37Packages.datasette)

@jonringer
Copy link
Contributor Author

@wd15 if the pypi package doesn't include tests, then I go to fetchFromGitHub route, as that definitely will have tests (if the they have tests at all)

@jonringer
Copy link
Contributor Author

@B4dM4n I was verifying that the expression had the needed dependencies and only the needed dependencies, having them in ad-hoc order wasn't fun. plus in vim, it's just a :!sort command away

@Mic92 Mic92 changed the title python3Packages.{websockets, uvicorn,python-engineio}: bump and fix builds python3Packages.{websockets, uvicorn,python-engineio}: bump an… Sep 20, 2019
@Mic92 Mic92 merged commit 0a56a46 into NixOS:master Sep 20, 2019
@Mic92
Copy link
Member

Mic92 commented Sep 20, 2019

[detached HEAD d3f56ac] python3Packages.websockets: 7.0 -> 8.0.2
Author: Jonathan Ringer jonringer117@gmail.com
Date: Mon Sep 16 13:24:14 2019 -0700
1 file changed, 11 insertions(+), 9 deletions(-)
Press Enter to continue
[detached HEAD 91abf95] python3Packages.uvicorn: 0.8.4 -> 0.9.0
Author: Jonathan Ringer jonringer117@gmail.com
Date: Mon Sep 16 15:07:28 2019 -0700
2 files changed, 16 insertions(+), 21 deletions(-)
delete mode 100644 pkgs/development/python-modules/uvicorn/setup.patch
Press Enter to continue
[detached HEAD 6825f04] python3Packages.python-engineio: 3.4.3 -> 3.9.3
Author: Jonathan Ringer jonringer117@gmail.com
Date: Mon Sep 16 13:29:34 2019 -0700
1 file changed, 12 insertions(+), 14 deletions(-)

@jonringer jonringer deleted the bump-websockets branch September 20, 2019 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@B4dM4n B4dM4n B4dM4n left review comments

@wd15 wd15 wd15 approved these changes

@FRidh FRidh Awaiting requested review from FRidh

@Mic92 Mic92 Awaiting requested review from Mic92

Assignees
No one assigned
Labels
6.topic: python 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jonringer @wd15 @Mic92 @B4dM4n