Skip to content

scrapy: init at 1.0.5#14878

Merged
joachifm merged 1 commit intoNixOS:masterfrom
drewkett:scrapy
Apr 22, 2016
Merged

scrapy: init at 1.0.5#14878
joachifm merged 1 commit intoNixOS:masterfrom
drewkett:scrapy

Conversation

@drewkett
Copy link

@drewkett drewkett commented Apr 21, 2016

Things done
  • Tested using sandboxing (nix-build --option build-use-chroot true or nix.useChroot on NixOS)
  • Built on platform(s)
    • NixOS
    • OS X
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

This adds scrapy and a couple of dependent python packages. I couldn't figure out how to build it using the sandbox, since its a part of the python-packages.nix file.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @zimbatm, @edolstra and @vcunat to be potential reviewers

@joachifm joachifm added the 6.topic: python Python is a high-level, general-purpose programming language. label Apr 21, 2016
@joachifm
Copy link
Contributor

How did you test it without building?

Also, please review the contribution guidelines.

@drewkett
Copy link
Author

I just started using nixos recently and don't yet understand how all of the
commands fit together. So I was able to build it but its not using the
chroot because I didn't understand how to use the nix-build command with a
python package

On Thu, Apr 21, 2016 at 3:18 PM Joachim Fasting notifications@github.com
wrote:

How did you test it without building?

Also, please review the contribution guidelines.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#14878 (comment)

@joachifm
Copy link
Contributor

@drewkett if you're on nixos, the best way is to enable nix.useChroot. To build a specific member of the python package set, do nix-build -A pythonPackages.myPackage.

@joachifm
Copy link
Contributor

The travis build log shows that the package is missing some inputs.

@drewkett
Copy link
Author

Thanks, that works for building it. I just added the pytest package since that's what travis says is missing. How do I get it to run the tests on my local machine, since I don't get those errors when building it?

@joachifm
Copy link
Contributor

What exactly are you doing to test this? The best approach is to configure the nix daemon to use sandboxing; this will reveal missing dependencies that would otherwise be picked up from your environment (which is probably the reason why you're not getting import errors).

@drewkett
Copy link
Author

I enabled nix.useChroot on my system, ran nixos-rebuild switch. And then ran nix-build -A pythonPackages.scrapy and it built just fine. The import error is from a missing module pytest which only comes up on testing the package, which must not have run when it was built. Does it run the tests when it builds the pacakge?

@drewkett
Copy link
Author

I was able to run nix-shell -p nox --run "nox-review wip" and everything seems to be working now. Thanks for the help. Let me know if there's anything else.

Copy link
Contributor

Choose a reason for hiding this comment

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

"This is a Python library" is redundant

@joachifm
Copy link
Contributor

Please squash when you're ready to merge

@drewkett
Copy link
Author

Done

@joachifm
Copy link
Contributor

LGTM. Thanks

@joachifm joachifm merged commit a240d95 into NixOS:master Apr 22, 2016
@Janik-Haag Janik-Haag added the 12.first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: python Python is a high-level, general-purpose programming language. 12.first-time contribution This PR is the author's first one; please be gentle!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants