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

pythonPackages.drmaa: init at 0.7.9 #50900

Closed
wants to merge 3 commits into from
Closed

Conversation

veprbl
Copy link
Member

@veprbl veprbl commented Nov 21, 2018

Motivation for this change
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)
  • Fits CONTRIBUTING.md.

@FRidh
Copy link
Member

FRidh commented Nov 22, 2018

@GrahamcOfBorg build pythonPackages.drmaa python3Packages.drmaa

@GrahamcOfBorg

This comment has been minimized.

@GrahamcOfBorg

This comment has been minimized.

@GrahamcOfBorg

This comment has been minimized.

@Ma27
Copy link
Member

Ma27 commented Nov 24, 2018

When I try to open this I get the following error message:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/nix/store/sxbibflcy8kl659mc2vxr7p8pkpy36bm-python-2.7.15-env/lib/python2.7/site-packages/drmaa/__init__.py", line 65, in <module>
    from .session import JobInfo, JobTemplate, Session
  File "/nix/store/sxbibflcy8kl659mc2vxr7p8pkpy36bm-python-2.7.15-env/lib/python2.7/site-packages/drmaa/session.py", line 39, in <module>
    from drmaa.helpers import (adapt_rusage, Attribute, attribute_names_iterator,
  File "/nix/store/sxbibflcy8kl659mc2vxr7p8pkpy36bm-python-2.7.15-env/lib/python2.7/site-packages/drmaa/helpers.py", line 36, in <module>
    from drmaa.wrappers import (drmaa_attr_names_t, drmaa_attr_values_t,
  File "/nix/store/sxbibflcy8kl659mc2vxr7p8pkpy36bm-python-2.7.15-env/lib/python2.7/site-packages/drmaa/wrappers.py", line 54, in <module>
    '{0}').format(_drmaa_lib_env_name))
RuntimeError: Could not find drmaa library.  Please specify its full path using the environment variable DRMAA_LIBRARY_PATH

As we aim to produce pure package builds I'd suggest to patch the software to ensure that libdrmaa can be found by the python module.

@veprbl
Copy link
Member Author

veprbl commented Nov 24, 2018

@Ma27
This is the intended behaviour. There is not a single implementation of "libdrmaa", it has to be implemented by a backend for a specific grid system and it needs to be provided separately. The setting to chose the specific backend is this DRMAA_LIBRARY_PATH environment variable. I plan to provide an expression for libcondordrmaa backend later, but in principle some other may come from existing batch system installation (possibly proprietary).

@Ma27
Copy link
Member

Ma27 commented Nov 24, 2018

I'm sorry, but I have to disagree with you here. I don't think that we want to have packages that are not functional by default.

An important part of providing a functional and reproducible setup for any given package (or environment) is that you have explicitly defined dependencies and don't rely on any global state (which may happen, especially on non-NixOS systems with Nix as additional package manager in your case).

Instead I'd propose the following: we package libcondordrmaa for nixpkgs and use this as default (and ensure this by patching the sources accordingly). In case somebody wants to use a different (maybe proprietary) implementation, he can override the expression like this: pythonPackages.drmaa.override { libcondordrmaa = proprietary_drmaa_impl; }. We may even want to name the parameter libdrmaa and use libcondordrmaa as default value (this would be more descriptive IMHO).

@veprbl
Copy link
Member Author

veprbl commented Nov 25, 2018

My plan for libcondordrmaa was to provide a "setupHook" that would set DRMAA_LIBRARY_PATH environment variable. That should provide a matching functionality without need to patch pythonPackages.drmaa.

I don't think that we want to have packages that are not functional by default.

Unfortunately simply nailing pythonPackages.drmaa to libcondordrmaa will not achieve this. The iceberg goes a bit further down to packaging HTCondor itself. Also, most people don't run their own batch systems, they use systems run by others (universities, government, private companies). Those systems usually run without any help of NixOS, so the purity of the environment will have some natural boundaries.

@Ma27
Copy link
Member

Ma27 commented Nov 25, 2018

My plan for libcondordrmaa was to provide a "setupHook" that would set DRMAA_LIBRARY_PATH environment variable. That should provide a matching functionality without need to patch pythonPackages.drmaa.

In the end this fairly similar to the approach I proposed earlier: in the end I want to have a sensitive default (that can be easily changed using the override mechanism in nixpkgs).

Unfortunately simply nailing pythonPackages.drmaa to libcondordrmaa will not achieve this. The iceberg goes a bit further down to packaging HTCondor itself. Also, most people don't run their own batch systems, they use systems run by others (universities, government, private companies). Those systems usually run without any help of NixOS, so the purity of the environment will have some natural boundaries.

I partially agree. Of course we can't ensure purity in distributed systems, especially if most (or all) of the systems don't run Nix(OS).

But here we're talking about some bindings to talk to any drmaa API using Python, right? And such a binding doesn't serve any purpose if there's no library to bind available.

The reason why I brought in explicit dependencies (for purity) is that I'd like to see a setup, where the user of any drmaa implementation using this bindings can specify which implementation (packaged with Nix) shall be used.

In this case we would've achieved the following goals:

  • Providing a working module by default
  • Avoiding misunderstandings when having several drmaa libraries (a global one, maybe managed by a different package manager) and one packaged with Nix in the build environment.

I guess I got your point and hope that I made my problem with this patch a bit clearer :)

@veprbl
Copy link
Member Author

veprbl commented Nov 27, 2018

@Ma27 Thank you for your continued interest. I think we are still not on the same page. Let me go through your points. You mention some of your preferred properties for pythonPackages.drmaa:

  1. To have a sensible default. Every grid system will have its own different backend, the user will have to explicitly choose it to match what is running in their system. Having libcondordrmaa enabled by default will do nothing for a user if user is given a Slurm or SGE, or PBS, or GridWay, or some other system. Also, I think, DRMAA specification targets quite a specific niche of problems. So most of the software that use this API will be custom-made and there is not much room for user's ignorance in the first place here.
  2. To have a working default. As I said before, I have no plans to implement HTCondor in NixOS, so having a default libcondordrmaa will not provide a working pythonPackages.drmaa.
  3. To avoid possible impurities. It is a common pattern in nixpkgs to have software rely on environment variables to operate. For example, the setupHook in python will set PYTHONPATH accordingly so that python packages can work. User still can override that variable and impurely use different modules if they want, we don't treat this case as impurity. What you propose looks more like python.withPackages to me. That is another idiom that we also use in nixpkgs. It is also not my favourite.

Overall I'm not against your approach. I have some reservations because I'm not sure how it would work to wrap proprietary backends for it. At this point I can't really tell which way would be better here.

@GrahamcOfBorg

This comment has been minimized.

@GrahamcOfBorg

This comment has been minimized.

@GrahamcOfBorg

This comment has been minimized.

@Ma27
Copy link
Member

Ma27 commented Nov 27, 2018

First of all thanks a lot for the detailed explanation!

Just to set this straight: there are a lot of (hard to package) implementations that can be used, to make the package actually usable you have to specify an environment variable which points to the actual library right? (sorry if I missed something, I never used this ecosystem before ^^)

If that's the case, I still have some concerns about encouraging users to use a different package not built with Nix (I'm referring to libdrmaa and HTCondor here) for the full setup. If you want to use the binding in a python-based environment managed by Nix you may want to place this in an overlay (IIRC we don't have hard rules for rejecting packages though).

It's certainly possible that we simply have different opinions if the above is the case, then I'd rather ask in the IRC what experienced maintainers think about this (I'm rather new here as maintainer, so I don't want to reject packages when the author has an understandable position I simply disagree with).

It is a common pattern in nixpkgs to have software rely on environment variables to operate. For example, the setupHook in python will set PYTHONPATH accordingly so that python packages can work. User still can override that variable and impurely use different modules if they want, we don't treat this case as impurity. What you propose looks more like python.withPackages to me. That is another idiom that we also use in nixpkgs. It is also not my favourite.

Agreed, I'm doing this myself in certain cases :)

But here you have a setup which is pure by default (explicitly defined Python packages living inside store paths). If a user modifies this (and maybe disables sandboxing) it's obviously possible to pull in additional (implicit) dependencies, but that's IMHO not what Nix was made for :)

@veprbl
Copy link
Member Author

veprbl commented Nov 28, 2018

Just to set this straight: there are a lot of (hard to package) implementations that can be used, to make the package actually usable you have to specify an environment variable which points to the actual library right? (sorry if I missed something, I never used this ecosystem before ^^)

Yes. The backends are dynamically linked libraries, this is specified by the DRMAA spec.

If that's the case, I still have some concerns about encouraging users to use a different package not built with Nix (I'm referring to libdrmaa and HTCondor here) for the full setup. If you want to use the binding in a python-based environment managed by Nix you may want to place this in an overlay (IIRC we don't have hard rules for rejecting packages though).

I would not say I encourage someone to have an impure setup. There is a way to package some of the backends (libcondordrmaa, the Slurm backend, maybe some others) and they will work because of the setupHook magic. Of course a user can implement an impure setup, say by executing

export DRMAA_LIBRARY_PATH=/opt/sge/lib/lx-amd64/libdrmaa.so

but is it that much different from using

pythonPackages.drmaa.override {
  libdrmaa = runCommand "libdrmaa.so" {}
                        "install -m444 /opt/sge/lib/lx-amd64/libdrmaa.so $out";
}

There is not much we can do to really prevent an impure setup.

But here you have a setup which is pure by default (explicitly defined Python packages living inside store paths). If a user modifies this (and maybe disables sandboxing) it's obviously possible to pull in additional (implicit) dependencies, but that's IMHO not what Nix was made for :)

Absolutely pure software is useless as it has no side effects. We are implementing as much purity as we can reasonably get. My proposed expression for pythonPackages.drmaa provides a package that can be built purely (even in sandbox) and operate within nixpkgs python ecosystem. This is a big deal. Sure, at runtime it will have to interact with some other components via DRMAA API. In a typical HPC environment some parts of those components will need to be borrowed from the existing system for practical reasons, they will be impure. We can also estimate a price of a mistake caused by such impurity. The underlying grid software may be updated by admins and that may break the library. Bummer! Will have to look how to fix that somehow. Now imagine a user invests time into preparing a 100% "pure" setup where all libraries versioned in the Nix store. An update of grid software can still break it by changing the network protocol used by the server. Purity here exchanges one set of problems for another. If neither is better why pay more?

@Ma27
Copy link
Member

Ma27 commented Nov 28, 2018

Absolutely pure software is useless as it has no side effects

I'm talking not about the software itself, I'm talking about the software build where we have explicitly defined dependencies and an isolated environment to provide a working build result in the end.

My point is that relying on an external, not explicitly defined package to make your python bindings work is needed and I think that's a problem (that's basically the detail I'm criticising) :)

But it's getting late now, please let me write an answer tomorrow :)

@Ma27
Copy link
Member

Ma27 commented Nov 28, 2018

but is it that much different from using

pythonPackages.drmaa.override {
  libdrmaa = runCommand "libdrmaa.so" {}
                       "install -m444 /opt/sge/lib/lx-amd64/libdrmaa.so $out";
}

That's as you mentioned not pure either. You don't know what exactly lives in /opt/..., so you're relying on an external dependency.

There is not much we can do to really prevent an impure setup.

The more I think about this, the more I agree with this.

As expressed previously I (personally) don't think that such a package should live in nixpkgs then and is far better suited for an overlay which contains derivations that are mainly suited for development purposes (and partly impure builds/build products).

I absolutely understand though if you disagree with me here, in that case let's ask an experienced maintainer, ok? :)

@GrahamcOfBorg

This comment has been minimized.

@GrahamcOfBorg

This comment has been minimized.

@GrahamcOfBorg

This comment has been minimized.

@veprbl veprbl mentioned this pull request Nov 29, 2018
10 tasks
@veprbl
Copy link
Member Author

veprbl commented Nov 30, 2018

So datrie is fixed, here we go again:

@GrahamcOfBorg build snakemake

@GrahamcOfBorg

This comment has been minimized.

@GrahamcOfBorg

This comment has been minimized.

@GrahamcOfBorg

This comment has been minimized.

@veprbl
Copy link
Member Author

veprbl commented Nov 30, 2018

Good

@GrahamcOfBorg build libcondordrmaa

@GrahamcOfBorg

This comment has been minimized.

@GrahamcOfBorg

This comment has been minimized.

@GrahamcOfBorg

This comment has been minimized.

@veprbl
Copy link
Member Author

veprbl commented Dec 1, 2018

@Ma27
So here is how I intended for this to be used:

nix-shell -p python3 -p python3Packages.drmaa -p libcondordrmaa \
  --run "python -c 'import drmaa; print(\"Success\")'"

@veprbl veprbl closed this Apr 9, 2019
@veprbl veprbl deleted the pr/drmaa_init branch December 1, 2020 16:55
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