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

pywb: init at 2.1.0; init pythonPackages.{warcio, surt, portalocker, certauth, wsgiprox, py3amf, redis_2, lupa, fakeredis} #52814

Closed
wants to merge 9 commits into from

Conversation

ivan
Copy link
Member

@ivan ivan commented Dec 25, 2018

Motivation for this change

Init pywb and its dependencies and subdependencies. pywb is one of the few tools for browsing the contents of WARC web archives.

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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

I verified on Linux and macOS that the WARC viewer works by following the instructions on https://pywb.readthedocs.io/en/latest/manual/usage.html#getting-started

wb-manager init my-archive
wb-manager add my-archive WARC_GZ_FILE
wayback -p 8081

Visit http://localhost:8081/my-archive/

Paste in a URL or domain present in the WARC file

Click a few times to navigate to the snapshot

@ivan
Copy link
Member Author

ivan commented Dec 25, 2018

Updated to use __darwinAllowLocalNetworking = true; on wsgiprox and pywb instead of disabling the tests on Darwin.

(via some interesting macOS sandbox code)

Copy link
Member

@FRidh FRidh left a comment

Choose a reason for hiding this comment

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

While fakeredis doesn't support the redis-py 3 interface, redis-py is supposedly backwards-compatible and so it can still be used with redis-py 3 to test the 2 interface. It's a bit unfortunate that they block this through install_requires. Using the following patch we don't need redis_2.

diff --git a/pkgs/development/python-modules/fakeredis/default.nix b/pkgs/development/python-modules/fakeredis/default.nix
index 83363a9a959..7e5a119ad1e 100644
--- a/pkgs/development/python-modules/fakeredis/default.nix
+++ b/pkgs/development/python-modules/fakeredis/default.nix
@@ -3,7 +3,7 @@
 , fetchPypi
 , lupa
 , pytest
-, redis_2
+, redis
 , nose
 }:
 
@@ -16,7 +16,11 @@ buildPythonPackage rec {
     sha256 = "005gnzj8lffz6hv5ii980gv8ffsiilqijdifyrz7lnms0c1852ms";
   };
 
-  propagatedBuildInputs = [ lupa redis_2 ];
+  postPatch = ''
+    substituteInPlace setup.py --replace "redis<3" "redis"
+  '';
+
+  propagatedBuildInputs = [ lupa redis ];
 
   checkInputs = [ pytest nose ];
 
@@ -24,7 +28,7 @@ buildPythonPackage rec {
   # `RuntimeError: dictionary changed size during iteration`
   # https://github.com/jamesls/fakeredis/issues/230
   checkPhase = ''
-    py.test -v -k "not test_pubsub_run_in_thread"
+    py.test -v -k "not test_pubsub_run_in_thread and not test_pipeline_as_context_manager"
   '';
 
   meta = with lib; {
diff --git a/pkgs/development/python-modules/pywb/default.nix b/pkgs/development/python-modules/pywb/default.nix
index e4a7e6f5980..d70eb1d7266 100644
--- a/pkgs/development/python-modules/pywb/default.nix
+++ b/pkgs/development/python-modules/pywb/default.nix
@@ -14,7 +14,7 @@
 , pyamf
 , pytest
 , pyyaml
-, redis_2
+, redis
 , requests
 , six
 , surt
@@ -95,7 +95,7 @@ buildPythonPackage rec {
   };
 
   propagatedBuildInputs = [
-    brotlipy chardet gevent jinja2 portalocker pytest pyyaml redis_2 requests
+    brotlipy chardet gevent jinja2 portalocker pytest pyyaml redis requests
     six surt warcio webassets webencodings werkzeug wsgiprox
     (if isPy3k then py3amf else pyamf)
   ];

@ivan
Copy link
Member Author

ivan commented Dec 25, 2018

Thanks for the review and the patch, updated to remove redis_2.

@FRidh
Copy link
Member

FRidh commented Dec 26, 2018

Good. Looks like pywb is an application, and not, or hardly, used as a library. We keep libraries in pkgs/development/python-modules and applications elsewhere in the tree. The Nixpkgs manual explains more about this.

@ivan ivan force-pushed the init-pywb branch 2 times, most recently from 0a51ed3 to d6144db Compare December 26, 2018 10:00
@ivan
Copy link
Member Author

ivan commented Dec 26, 2018

Good catch, thanks, I moved it out of python-modules.

@ivan
Copy link
Member Author

ivan commented Jan 1, 2019

non-urgent ping, let me know if I need to do anything else

Use lua instead of luajit because of problems on Darwin:

  File "/private/var/folders/ym/xnfgyq_n2fzgzzx9gtccxdjm0000gn/T/nix-build-python3.7-lupa-1.7.drv-0/lupa-1.7/lupa/tests/test.py", line 29, in setUp
    self.lua = lupa.LuaRuntime(**self.lua_runtime_kwargs)
  File "lupa/_lupa.pyx", line 208, in lupa._lupa.LuaRuntime.__cinit__
    raise LuaError("Failed to initialise Lua runtime")
lupa._lupa.LuaError: Failed to initialise Lua runtime
@ivan
Copy link
Member Author

ivan commented Feb 13, 2019

I removed the commit that skipped the dateparser tests, now that it's been fixed for Python 3.7.

@mmahut
Copy link
Member

mmahut commented Aug 20, 2019

Are there any updates on this pull request, please?

@ivan
Copy link
Member Author

ivan commented Aug 20, 2019

This was unfortunately never merged in time to avoid conflicts.

Also, I don't use pywb right now, but if I start using it, I'll submit another PR.

@ivan ivan closed this Aug 20, 2019
@anpandey anpandey mentioned this pull request Sep 6, 2022
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants