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

pythonPackages.webargs: init at 6.1.0 #91703

Merged
merged 3 commits into from Jun 24, 2021
Merged

pythonPackages.webargs: init at 6.1.0 #91703

merged 3 commits into from Jun 24, 2021

Conversation

cript0nauta
Copy link
Contributor

@cript0nauta cript0nauta commented Jun 28, 2020 •
edited

Motivation for this change

I added the webargs and webtest-aiohttp Python packages. Also, removed some unused dependencies from marshmallow (otherwise the webargs test would fail).

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.

@cript0nauta
Copy link
Contributor Author

@jonringer thanks for all the suggestions! I applied them all and rebased the branch.

cript0nauta added a commit to cript0nauta/pynixify that referenced this pull request Aug 5, 2020
@cript0nauta
Add some cosmetic changes to the generated expressions
09cece9 
See NixOS/nixpkgs#91703
@stale
Copy link

stale bot commented Jun 6, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 6, 2021
@jonringer
Copy link
Contributor

@cript0nauta still interested in this? sorry it got backburnered

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 7, 2021
SuperSandro2000
SuperSandro2000 requested changes Jun 9, 2021
View reviewed changes
pkgs/development/python-modules/webargs/default.nix Outdated
Comment on lines 36 to 37
description =
"Declarative parsing and validation of HTTP request objects, with built-in support for popular web frameworks";
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
description =
"Declarative parsing and validation of HTTP request objects, with built-in support for popular web frameworks";
description = "Declarative parsing and validation of HTTP request objects, with built-in support for popular web frameworks";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. BTW, I used nixfmt to format this file, that's why it had an extra linebreak.

pkgs/development/python-modules/webargs/default.nix Outdated

propagatedBuildInputs = [ marshmallow ];
checkInputs = [
pytest
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
pytest
pytestCheckHook

pkgs/development/python-modules/webargs/default.nix Outdated
Comment on lines 15 to 18
checkPhase = ''
rm tests/test_webapp2parser.py # webapp2 doesn't support python 3
pytest
'';
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
checkPhase = ''
rm tests/test_webapp2parser.py # webapp2 doesn't support python 3
pytest
'';
preCheck = ''
rm tests/test_webapp2parser.py # webapp2 doesn't support python 3
'';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file was already removed in webargs 8.0.0, so there is no need to remove it from the Nix expressoin.

pkgs/development/python-modules/webargs/default.nix Outdated
@@ -0,0 +1,42 @@
{ buildPythonPackage, fetchPypi, lib, isPy27, marshmallow, pytest
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
{ buildPythonPackage, fetchPypi, lib, isPy27, marshmallow, pytest
{ buildPythonPackage, fetchPypi, lib, isPy27, marshmallow, pytestCheckHook

pkgs/development/python-modules/webtest-aiohttp/default.nix Outdated
};

checkPhase = ''
invoke test
Copy link
Member

Choose a reason for hiding this comment

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

Can we use pytestCheckHook instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

pkgs/development/python-modules/webtest-aiohttp/default.nix Outdated
Comment on lines 24 to 25
description =
"Provides integration of WebTest with aiohttp.web applications";
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
description =
"Provides integration of WebTest with aiohttp.web applications";
description = "Provides integration of WebTest with aiohttp.web applications";

pkgs/development/python-modules/marshmallow/default.nix Outdated
@@ -21,7 +19,5 @@ buildPythonPackage rec {
sha256 = "35ee2fb188f0bd9fc1cf9ac35e45fd394bd1c153cee430745a465ea435514bd5";
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind adding yourself as a maintainer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@cript0nauta cript0nauta force-pushed the webargs branch 2 times, most recently from dbb32fc to a57a292 Compare June 18, 2021 07:16
@cript0nauta
Copy link
Contributor Author

Rebased, upgraded to webargs 8 and fixed suggestions.

SuperSandro2000
SuperSandro2000 requested changes Jun 18, 2021
View reviewed changes
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Please add pythonImportsChecks.

@cript0nauta
Copy link
Contributor Author

Please add pythonImportsChecks.

Done

@SuperSandro2000
Copy link
Member

I forgot to mention that the last time but please squash the commits into one per package.

@cript0nauta
Copy link
Contributor Author

Sure, done!

@SuperSandro2000 SuperSandro2000 merged commit a6dfcab into NixOS:master Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer jonringer left review comments

@SuperSandro2000 SuperSandro2000 SuperSandro2000 requested changes

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@cript0nauta @jonringer @SuperSandro2000 @ryantm