pythonPackages.connexion

[elohmeier]

Motivation for this change

add the connexion library and dependencies to build nice Flask-API's with Nix

Things done

added clickclick, openapi-spec-validator, connexion

• 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.

---

[worldofpeace]

I'm seeing you've disabled tests in all of these packages because they aren't distributed from PyPI.

Usually in that case I'd recommend not fetching from PyPI and instead using the github source with fetchFromGitHub.

Also any dependencies needed for tests should be in checkInputs.

[elohmeier]

@worldofpeace thanks for your feedback! I've switched the packages to fetch from GitHub. In some cases the tests fail, so I've disabled them - but they can be executed now. I've also updated the wrong link and added another optional dependency for connexion.

[elohmeier]

@worldofpeace thanks for your helpful feedback! I really appreciate it =)
Thanks for the tip with pytest -k, that worked out. I've updated the packages.

[elohmeier]

@worldofpeace thanks for your feedback =)

[worldofpeace]

@elohmeier I'm seeing that they're some deps that are already in propagatedBuildInputs that are needed for testing that aren't in checkInputs in connexion.

I'm not sure if that was on purpose, but they should be declared as checkInputs if we need them for tests.

Also I think we're missing decorator.

#52580 (comment)
I've modified that part as you suggested. However there are a lot of unit tests failing if I'm trying to build it for Python 2.7 or Python 3.6. Should we disable the tests for that versions?

Can you paste the logs somewhere after this round of review?
Perhaps we're missing something.

[elohmeier]

@worldofpeace thanks again for your review! I've been able to make the package build and test successfully now for py27, py35, py36 and py37. There was a problem using the wrong locale for Python <3.7 (seems to be the same as nix-community/pypi2nix#52, took the fix from there). I've also added the missing dependecies (adding another package).
There is still a problem though, the authors of connexion need aiohttp<3.5.2. I've locally switched to 3.5.1, but nixpkgs has a more recent version on master which makes the tests fail. Do you have a suggestion on how to handle that?
Your feedback is much appreciated :-)

[worldofpeace]

There is still a problem though, the authors of connexion need aiohttp<3.5.2. I've locally switched to 3.5.1, but nixpkgs has a more recent version on master which makes the tests fail. Do you have a suggestion on how to handle that?
Your feedback is much appreciated :-)

Looks like they pinned it because the newer version broke their build spec-first/connexion#844.
Might want to bug them to fix that soon.

We could skip the aiohttp tests and hope it isn't broken because of it or we could pin the version within the expression.

And you could do that like

let

  aiohttp_3_5_1 = aiohttp.overridePythonAttrs (oldAttrs: {
    version = "3.5.1";

    src = fetchPypi {
      inherit (oldAttrs) pname;
      inherit version;
      sha256 = ...;
    };
  });

in

[worldofpeace]

Actually you can't override it because its dependents already have aiohttp in their propagatedBuildInputs and we simply can't have that ever because it will conflict.

[mmahut]

Are there any updates on this pull request, please?

[elohmeier]

@worldofpeace Thanks for your feedback, I've updated my changes accordingly. I've also updated to the latest package versions.

[worldofpeace]

@elohmeier, we've actually undergone an API change for python packaging since this change was authored.

We have a setup hook for running pytest

• f7e28bf

so wherever a package uses pytest you should migrate like this

checkInputs = [
  pytestCheckHook # you should drop pytest from checkInputs as well
];

disabledTests = [
  "some_disabled_test_case" # uses -k internally etc.
];

[elohmeier]

@worldofpeace Thanks for the review and info, I've switched to the new API where possible.
I've also introduced a patch to deal with the aiohttp problem from upstream, which led to no regression in the enabled tests.

[worldofpeace]

@worldofpeace Thanks for the review and info, I've switched to the new API where possible.
I've also introduced a patch to deal with the aiohttp problem from upstream, which led to no regression in the enabled tests.

Great, I'm going to build everything locally. Pretty sure we should be good to merge 👍

[worldofpeace]

Thanks @elohmeier ✨