python3Packages.gradio: init at 3.20.1 #241558

pbsds · 2023-07-04T21:26:18Z

Description of changes

This PR adds gradio by @gradio-app, a web server framework for making AI (anything really) demos.

Original PR is #187032, which was closed due to a mass-ping resulting from a poorly thought out rebase onto staging.

Close #235184

Things done

Pinging reviewers from prior pr:
@happysalada @Janik-Haag @VergeDX @SuperSandro2000

pbsds · 2023-07-04T21:30:06Z

@ofborg build python3Packages.gradio

happysalada · 2023-07-05T00:28:17Z

@mweinelt the ci fails on aarch64-linux, the logs don't seem to say much. This could be due to a timeout. Is this of any concern to you ?

happysalada · 2023-07-06T00:14:42Z

@pbsds this looks good to me, do you intend to do any more work on this ?

happysalada · 2023-07-13T11:33:29Z

@pbsds I'm waiting for this to package FastChat, let me know if I can do anything to help.

mweinelt

The test phase of gradio looks annyoing to maintain.

pkgs/development/python-modules/ffmpy/default.nix

pkgs/development/python-modules/markdown-it-py/default.nix

pkgs/development/python-modules/gradio/default.nix

happysalada · 2023-07-20T11:55:46Z

@pbsds hey, it looks like you are a little busy, would you mind If I took over ?

pbsds · 2023-07-20T16:05:29Z

@pbsds hey, it looks like you are a little busy, would you mind If I took over ?

My computer and network access this summer is spotty. I added you to my nixpkgs repo if you're interested in joint maintainership :)

@ofborg build python3Packages.gradio

happysalada · 2023-07-21T19:31:10Z

This looks good to me.
Im happy to be pinged in case there are problems discovered during the staging PR.
@mweinelt might have more comments, so waiting a bit more.

pkgs/development/python-modules/ffmpy/default.nix

mweinelt · 2023-07-22T00:10:29Z

pkgs/development/python-modules/gradio/default.nix

+  # Add pytest hook skipping tests that access network, marking them as "Expected fail" (xfail)
+  # We additionally xfail FileNotFoundError, as gradio often fail to upload test files to pypi.
+  preCheck = let
+    conftestSkipNetworkErrors = writeText "conftest-network-xfail.py" ''
+      from _pytest.runner import pytest_runtest_makereport as orig_pytest_runtest_makereport
+      import urllib, urllib3, httpx, requests, websockets
+
+      class NixNetworkAccessDeniedError(BaseException): pass
+      def deny_network_access(*a, **kw):
+        raise NixNetworkAccessDeniedError
+
+      def iterate_exc_chain(exc: Exception):
+        yield exc
+        if getattr(exc, "__context__", None) is not None:
+          yield from iterate_exc_chain(exc.__context__)
+        if getattr(exc, "__cause__", None) is not None:
+          yield from iterate_exc_chain(exc.__cause__)
+
+      httpx.Request = \
+      httpx.AsyncClient.get = \
+      httpx.AsyncClient.head = \
+      requests.head = \
+      requests.get = \
+      requests.post = \
+      urllib.request.urlopen = \
+      urllib.request.Request = \
+      urllib3.connection.HTTPSConnection._new_conn = \
+      websockets.connect = \
+        deny_network_access
+
+      def pytest_runtest_makereport(item, call):
+        tr = orig_pytest_runtest_makereport(item, call)
+        if call.excinfo is not None:
+          for exc in iterate_exc_chain(call.excinfo.value):
+            if isinstance(exc, NixNetworkAccessDeniedError):
+              tr.outcome, tr.wasxfail = 'skipped', "reason: Requires network access."
+            if isinstance(exc, FileNotFoundError):
+              tr.outcome, tr.wasxfail = 'skipped', "reason: Pypi dist bad."
+        return tr
+    '';
+  in ''
+    export HOME=$TMPDIR
+    cat ${conftestSkipNetworkErrors} >> test/conftest.py
+  '';


Is some of that upstreamable? Like a network marker? As I mentioned, this looks annoying to maintain.

IMO this is too related to nix to matter upstream.
How about we keep this as is, and if for some reason this doesn't work anymore, we just disable the tests until someone has time to look into this.
Since there are quite a lot of packages depending on gradio, would you rather

disable the tests until we confirm with upstream if they are interested in this or not.

leave this as is for now and disable the tests if there is a failure related to this.

IMO this is too related to nix to matter upstream.

Having an extra marker to skip network tests is something not nix specific, many distros build their packages without networking. It can also be a great way to skip tests if no network is available or only very slow and flaky.

In the prior pr I discussed generalizing this into a pytestCheckHook-esqe hook. The sentiment then was first pushing through this pr

If the usecase comes up, we can always move the implementation into a hook but I am not fully convinced that this is necessary here.

What's the problem with upstreaming this and adding a marker to skip network tests? We just have a build environment without internet access but we would still like to run as many tests as possible.

I don't trust the gradio devs, who tend to move fast and loose, to support this use case. My reason is that i did this pr, to fix the missing test files on pypi. They they promptly broke it again within a few weeks.

I have bumped the version of the prior gradio pr about 3 times over the past year, and each time the list of tests to disable needed change. This proved quite time consuming to maintain (hence that debug helper comment). Network and non-network tests are not grouped in any meaningful sense, so disabledTestPaths proved too aggressive while disabledTests proved too surgical and unmaintainable.

The conftest network xfail approach came upon me in a dream as an easily generalizable drop-in way to run as many tests as possible, without maintaining a disabledTests list, nor relying on upstream devs to support us poor distro maintainers. It is my brain child and I will defend it with passion :P.

To argue in favor of generalizing this xfail approach and a future check hook, here is a very coarse estimate of the number of packages which disable network tests, and likely a lot more, with disableTestPaths:

$ rg disabledTestPaths pkgs/development/python-modules/ -l | xargs rg network | wc -l 137

The conftest network xfail approach will for these likely improve testing coverage by a lot, and I believe such a hook will even remove a lot of doChecks = false; found in throughout our python package set.

pkgs/top-level/python-packages.nix

happysalada · 2023-07-23T15:21:38Z

@mweinelt ready for another round of review.
I've added two commits that will later be squashed. I didn't want to replace the commits so that the OP would loose attribution.
let me know if anything.

pkgs/top-level/python-packages.nix

pkgs/development/python-modules/ffmpy/default.nix

pkgs/development/python-modules/markdown-it-py/default.nix

SuperSandro2000 · 2023-07-23T17:57:33Z

pkgs/development/python-modules/gradio/default.nix

+    # makes pytest freeze 50% of the time
+    "test/test_interfaces.py"
+  ];
+  #pytestFlagsArray = [ "-x" "-W" "ignore" ]; # uncomment for debugging help


Can't we enable this by default to better debug hydra builds immediately? Would that be to spammy?

if we start getting failures on the package, I would say let's enable it. For now it's best to leave it disabled I think.

SuperSandro2000 · 2023-07-23T18:13:57Z

pkgs/development/python-modules/gradio/default.nix

+  # Add pytest hook skipping tests that access network, marking them as "Expected fail" (xfail)
+  # We additionally xfail FileNotFoundError, as gradio often fail to upload test files to pypi.
+  preCheck = let
+    conftestSkipNetworkErrors = writeText "conftest-network-xfail.py" ''
+      from _pytest.runner import pytest_runtest_makereport as orig_pytest_runtest_makereport
+      import urllib, urllib3, httpx, requests, websockets
+
+      class NixNetworkAccessDeniedError(BaseException): pass
+      def deny_network_access(*a, **kw):
+        raise NixNetworkAccessDeniedError
+
+      def iterate_exc_chain(exc: Exception):
+        yield exc
+        if getattr(exc, "__context__", None) is not None:
+          yield from iterate_exc_chain(exc.__context__)
+        if getattr(exc, "__cause__", None) is not None:
+          yield from iterate_exc_chain(exc.__cause__)
+
+      httpx.Request = \
+      httpx.AsyncClient.get = \
+      httpx.AsyncClient.head = \
+      requests.head = \
+      requests.get = \
+      requests.post = \
+      urllib.request.urlopen = \
+      urllib.request.Request = \
+      urllib3.connection.HTTPSConnection._new_conn = \
+      websockets.connect = \
+        deny_network_access
+
+      def pytest_runtest_makereport(item, call):
+        tr = orig_pytest_runtest_makereport(item, call)
+        if call.excinfo is not None:
+          for exc in iterate_exc_chain(call.excinfo.value):
+            if isinstance(exc, NixNetworkAccessDeniedError):
+              tr.outcome, tr.wasxfail = 'skipped', "reason: Requires network access."
+            if isinstance(exc, FileNotFoundError):
+              tr.outcome, tr.wasxfail = 'skipped', "reason: Pypi dist bad."
+        return tr
+    '';
+  in ''
+    export HOME=$TMPDIR
+    cat ${conftestSkipNetworkErrors} >> test/conftest.py
+  '';


IMO this is too related to nix to matter upstream.

Having an extra marker to skip network tests is something not nix specific, many distros build their packages without networking. It can also be a great way to skip tests if no network is available or only very slow and flaky.

pkgs/development/python-modules/gradio/default.nix

pkgs/development/python-modules/ffmpy/default.nix

pkgs/development/python-modules/gradio/default.nix

pkgs/development/python-modules/ffmpy/default.nix

pkgs/development/python-modules/gradio/default.nix

happysalada · 2023-07-24T01:22:31Z

@SuperSandro2000 I think I've addressed all of your comments, but let me know if there is something that you think should be addressed differently.

I've also just tested to make sure this actually works.

Janik-Haag · 2023-08-07T11:56:21Z

Are you sure the package even works if there are so many test failures?

happysalada · 2023-08-07T12:28:31Z

First step is merging i think. Trying to rebuild all staging everytime to test us just not very efficient. First we make sure nothing breaks, then we should be able toake changes that impact much less things. I have locally packaged fast chat, ill open a PR as soon as this hits master.

Janik-Haag · 2023-08-07T12:37:26Z

You can just do git switch master and then cherry pick the commits like this: git cherry-pick fdfcecedb72326c19772b4720a2e9c6df46347db 4324abf898a0ff5a14fe4b740f81f97ddc2e34a8 90500b167084b87a385c68f29de128418bc2da35 if you checked out this branch locally with gh pr checkout 241558. and then you can test locally without problem

pbsds · 2023-08-07T12:43:40Z

I'm unable to reproduce your error on master. I also tried python 3.11, but that was blocked by tensorflow being marked broken on master. I'm disabling test_in_interface due to being flaky.

Are you sure the package even works if there are so many test failures?

The gradio devs tend to move fast and break things, and the functionality they provide is incredibly simple but pimped with lots of fancy webdev. The gradio test suite is mostly integration testing with all the major ML frameworks, hence the ginormous check closure.

Perhaps the future is doCheck = false, but as of now i consider this testing coverage a major success

pbsds · 2023-08-07T12:48:50Z

You can just do git switch master and then cherry pick the commits like this: git cherry-pick fdfcecedb72326c19772b4720a2e9c6df46347db 4324abf898a0ff5a14fe4b740f81f97ddc2e34a8 90500b167084b87a385c68f29de128418bc2da35 if you checked out this branch locally with gh pr checkout 241558. and then you can test locally without problem

I use this one-liner, which assumes this pr is checked out in the worktree ../gradio-init:

git checkout master;  (cd ../gradio-init/; git log --oneline) | head -n3 | cut -d\  -f1 | tac | xe git cherry-pick

I'm surprised how nixpkgs-review does not have a cherry-pick or rebase strategy for staging PRs. (Mic92/nixpkgs-review#287)

happysalada · 2023-08-07T13:00:16Z

@Janik-Haag have your concerns been addressed ?

Regarding my test failure, ive had other test failures on staging that couldbt be reproduced. I have had one on thrift, that was tested on another machine and couldnt be reproduced. So the one i had could be just a fluke.

Janik-Haag · 2023-08-07T13:17:51Z

I currently am in a train so I have whonky internet, I'll try to check the package in the next ~8 hours once I have a stable connection again. But I really don't want to merge anything broken.

happysalada · 2023-08-07T13:23:16Z

At this point, i dont think this paclage is broken, i think there could be flakiness in the tests or some potential problems with downstream dependents, but am not sure how we much more we could test.

Happy to wait one more day if you think we can potentially get more information.

Co-authored-by: Yt <[email protected]>

linkify-it-py is on version 2.0.0, while markdown-it-py[linkify] depends on linkify-it-py~=1.0

pbsds · 2023-08-07T14:42:56Z

I rebased onto the recent merge base between master and staging, and now it builds. So far I've waited one hour for the ofborg eval to be done. I'll marking the PR ready again once the eval is done, just for safety.

pbsds · 2023-08-07T15:03:34Z

@ofborg build python3Packages.gradio

happysalada · 2023-08-07T16:45:24Z

Ive built this on my machine and it worked like a charm this time.

Janik-Haag · 2023-08-07T20:21:41Z

It now builds fine but when I try to run the binary it throws this error:

./result/bin/gradio
Traceback (most recent call last):
  File "/nix/store/3nap39bl8f7wbrbzfkl1dbj15lnab8aj-python3.10-gradio-3.20.1/bin/.gradio-wrapped", line 6, in <module>
    from gradio.reload import run_in_reload_mode
  File "/nix/store/3nap39bl8f7wbrbzfkl1dbj15lnab8aj-python3.10-gradio-3.20.1/lib/python3.10/site-packages/gradio/__init__.py", line 3, in <module>
    import gradio.components as components
  File "/nix/store/3nap39bl8f7wbrbzfkl1dbj15lnab8aj-python3.10-gradio-3.20.1/lib/python3.10/site-packages/gradio/components.py", line 33, in <module>
    from gradio.blocks import Block, BlockContext
  File "/nix/store/3nap39bl8f7wbrbzfkl1dbj15lnab8aj-python3.10-gradio-3.20.1/lib/python3.10/site-packages/gradio/blocks.py", line 24, in <module>
    from gradio import (
  File "/nix/store/3nap39bl8f7wbrbzfkl1dbj15lnab8aj-python3.10-gradio-3.20.1/lib/python3.10/site-packages/gradio/networking.py", line 17, in <module>
    from gradio.routes import App
  File "/nix/store/3nap39bl8f7wbrbzfkl1dbj15lnab8aj-python3.10-gradio-3.20.1/lib/python3.10/site-packages/gradio/routes.py", line 25, in <module>
    import pkg_resources
ModuleNotFoundError: No module named 'pkg_resources'

Co-authored-by: Yt <[email protected]>

pbsds · 2023-08-08T02:37:04Z

Great catch! I fixed it, and also added a postInstallCheck testing for this.
I'm surprised the pythonImportsCheck runs in the python stdenv.

I hosted a gradio service last year, so it didn't strike me to check the shell entrypoint as I kept bumping the version of the prior PR.

Janik-Haag

LGTM now, sorry for all the trouble but I want to avoid merging broken stuff.

happysalada · 2023-08-08T10:43:23Z

@Janik-Haag on the contrary, thank you for your thoroughness.

@pbsds thanks for sticking with this PR for so long!

Magellol · 2024-03-29T00:07:00Z

Unfortunately, It doesn't look like I can build this on aarch64-darwin :/ — Anyone has an idea? I'm not super knowledgeable with the ins and outs of python.

error: builder for '/nix/store/j5m87whdmc1ryjy21l99r0rinf3rzmlz-python3.11-gradio-client-0.10.1.drv' failed with exit code 4;
       last 10 log lines:
       > /nix/store/9nasxys60wirmhqs07akfb68n83pip6x-python3.11-gradio-sans-client-4.20.1/lib/python3.11/site-packages/gradio/routes.py:81: in <module>
       >     from gradio.server_messages import (
       > /nix/store/9nasxys60wirmhqs07akfb68n83pip6x-python3.11-gradio-sans-client-4.20.1/lib/python3.11/site-packages/gradio/server_messages.py:64: in <module>
       >     class UnexpectedErrorMessage(BaseModel):
       > /nix/store/9nasxys60wirmhqs07akfb68n83pip6x-python3.11-gradio-sans-client-4.20.1/lib/python3.11/site-packages/gradio/server_messages.py:65: in UnexpectedErrorMessage
       >     msg: Literal[ServerMessage.unexpected_error] = ServerMessage.unexpected_error
       > /nix/store/c05vbvkjxarxkws9zkwrcwrzlsx9nd68-python3-3.11.8/lib/python3.11/enum.py:784: in __getattr__
       >     raise AttributeError(name) from None
       > E   AttributeError: unexpected_error
       > /nix/store/gamm276nsv0l0qm71qhywpz833niaxkx-stdenv-darwin/setup: line 1587: pop_var_context: head of shell_variables not a function context

My flake looks like this

{
  description = "A flake for pythonification";

  inputs = {
    # Ensure our nixpkgs revision is green in Hydra for both Linux and macOS (i.e. there are
    # prebuilt binaries in the cache):
    #   https://status.nixos.org
    nixpkgs.url = "github:nixos/nixpkgs/nixpkgs-unstable";
    flake-utils.url = "github:numtide/flake-utils";
  };

  outputs = { nixpkgs, flake-utils, ... }: flake-utils.lib.eachDefaultSystem (system:
    let
      pkgs = import nixpkgs {
        inherit system;
      };
    in rec {
      devShell = pkgs.mkShell {
        buildInputs = with pkgs; [
          (python3.withPackages(ps: with ps; [
            gradio-client
          ]))
        ];
      };
    }
  );
}

pbsds · 2024-03-29T14:55:46Z

I'm sorry to say that I'm unable to test on mac.

The failure is expected

The error printout you provided cuts of too early, but hydra did get the same error. It fails to read this attribute, which makes no sense. It may be a circular import problem where utils is only partially evaluated, and this i cannot fix trivially

Consider overriding the package with doCheck = False, or opening an issue

github-actions bot added the 6.topic: python label Jul 4, 2023

pbsds changed the title ~~Gradio init~~ python3Packages.gradio: init at 3.20.1 Jul 4, 2023

ofborg bot added the 8.has: package (new) This PR adds a new package label Jul 4, 2023

ofborg bot requested a review from bhipple July 4, 2023 23:22

ofborg bot added 10.rebuild-darwin: 501+ 10.rebuild-darwin: 1001-2500 10.rebuild-linux: 501+ 10.rebuild-linux: 1001-2500 labels Jul 4, 2023

mweinelt requested changes Jul 13, 2023

View reviewed changes

pbsds force-pushed the gradio-init branch from e2cd3cf to 12437f8 Compare July 20, 2023 16:00

pbsds requested a review from happysalada July 20, 2023 16:07

mweinelt requested changes Jul 22, 2023

View reviewed changes

happysalada reviewed Jul 23, 2023

View reviewed changes

pkgs/top-level/python-packages.nix Outdated Show resolved Hide resolved

SuperSandro2000 reviewed Jul 23, 2023

View reviewed changes