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

[WIP] Update Azure sdk #63612

Closed
wants to merge 23 commits into from
Closed

Conversation

jonringer
Copy link
Contributor

Motivation for this change

Author for #60435 hasn't been heard from for almost a month.

Had to pin azure-storage for nixops_1_6_1.

Eventually I hope to fix the pythonPackages.azure, as it's currently horribly broken. Also hope to restore proper support for azure in nixops (but that will be much later)

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 nix-review --run "nix-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.

@jonringer
Copy link
Contributor Author

jonringer commented Jun 21, 2019

Unfortunately the azure-mgmt package doesn't include the latest major bumps of the packages, so I had to revert them back to the previous versions

@jonringer
Copy link
Contributor Author

quoted url's and rebased off master to avoid merge conflicts

@jonringer
Copy link
Contributor Author

@FRidh what's your opinion on making a buildAzurePythonPackage and buildAzureMgmtPythonPackage where I make all the namespace changes generically over the packages. Getting really tired of all the postPatch and postInstall hooks.

@kalbasit
Copy link
Member

Are you planning to include all the changes from #60435?

@FRidh
Copy link
Member

FRidh commented Jul 16, 2019

@jonringer please write a function to perform overridePythonAttrs instead. That way we can guarantee that overriding will still work. Downside is that your overrides take precedence over what is in the original expression.

Has upstream considered fixing their namespace packages?

@jonringer
Copy link
Contributor Author

jonringer commented Jul 16, 2019

@kalbasit eventually yes

@FRidh AFAIK, upstream is waiting for python2 to be phased out and then they can remove their azure-nspkg "work-around"

for the buildHelpers, I was thinking I can do something to the effect of a pass thru function which would default the postInstall and postPatch phases similar to:

pkgs/top-level/python-packages.nix

  buildAzurePythonPackage =
    let
      # remove python2 vs python3 namespace hacks
      postPatch' = ''
        sed '/azure-namespace-package/d' setup.py
        rm -f azure_bdist_wheel.py
      '';
      postInstall' = (if isPy3k then ''
        rm -f "$out/lib/${python.sitePackages}"/azure/__init__.py
      '' else ''
        echo "__import__('pkg_resources').declare_namespace(__name__)" > "$out/lib/${python.sitePackages}"/azure/__init__.py
      '');
    in
      { postPatch ? postPatch'
      , postInstall ? postInstall', ...}@attrs:
        buildPythonPackage attrs;

then a package would look like:

{ buildAzurePythonPackage
, fetchPypi
, isPy3k
, lib
, python
, azure-common
, msrestazure
}:

buildAzurePythonPackage rec {
  version = "4.1.3"; # don't bump version until next bump of azure umbrella package
  pname = "azure-batch";

  src = fetchPypi {
    inherit pname version;
    extension = "zip";
    sha256 = "0k1n73qpa2ns2kjsf9rjsv4b0vcdw6dggfr5c95igaxynpmwfwfd";
  };

  propagatedBuildInputs = [
    azure-common
    msrestazure
  ];

  # tests not included in source package
  # github repo contains over 80 other azure namespace packages
  doCheck = false;

  meta = with lib; {
    description = "Microsoft Azure SDK for Python";
    homepage = "https://github.com/Azure/azure-sdk-for-python";
    license = licenses.mit;
    maintainers = with maintainers; [ jonringer ];
  };
}

instead of the previous

{ buildPythonPackage
, fetchPypi
, isPy3k
, lib
, python
, azure-common
, msrestazure
}:

buildPythonPackage rec {
  version = "4.1.3"; # don't bump version until next bump of azure umbrella package
  pname = "azure-batch";

  src = fetchPypi {
    inherit pname version;
    extension = "zip";
    sha256 = "0k1n73qpa2ns2kjsf9rjsv4b0vcdw6dggfr5c95igaxynpmwfwfd";
  };

  propagatedBuildInputs = [
    azure-common
    msrestazure
  ];

  # Prevent package from doing unusual wheel build from source
  postPatch = ''
    substituteInPlace setup.cfg \
      --replace "azure-namespace-package = azure-nspkg" ""
    rm azure_bdist_wheel.py
  '';

  # Ahere to PEP420 for python3
  postInstall = lib.optionalString isPy3k ''
    rm "$out/lib/${python.libPrefix}"/site-packages/azure/__init__.py
  '';

  # tests not included in source package
  # github repo contains over 80 other azure namespace packages
  doCheck = false;

  meta = with lib; {
    description = "Microsoft Azure SDK for Python";
    homepage = "https://github.com/Azure/azure-sdk-for-python";
    license = licenses.mit;
    maintainers = with maintainers; [ jonringer ];
  };
}

across 80 or so packages, this will help a lot :)

@jonringer
Copy link
Contributor Author

There would be a similar function for the azure-mgmt-* packages as well

@jonringer
Copy link
Contributor Author

Also, for the packages that have a high major version than what the azure meta package has, I was think of moving those expression under the python-modules/azure/ directory and then just adding the latest to the python-packages.nix

@kalbasit
Copy link
Member

kalbasit commented Jul 16, 2019

@jonringer I applied all the changes in #60435 and rebased it off of master. Can you please give that PR a try, I'd like to get it merged soon/today if possible.

I like the idea of having a common overlay we can apply to all packages, can this PR be built on top of #60435 once it lands on master to DRY up the code?

@jonringer
Copy link
Contributor Author

I can once i get home. Just on lunch right now.

@kalbasit
Copy link
Member

I can once i get home. Just on lunch right now.

I will keep an eye out for your comment then. Bon Appetit!

@jonringer
Copy link
Contributor Author

closing in favor of #71797

@jonringer jonringer closed this Oct 31, 2019
@jonringer jonringer deleted the update_azure_sdk branch October 31, 2019 08:11
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