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.mlflow: init at 1.4.0 #74091
Conversation
Here's one issue I'm stuck on:
This appears to be benoitc/gunicorn#1716. So I tried adding Also possibly related to #68314. Anyone know how to fix this? |
I originally was going to post this in tl;dr why are python3.7 paths being appended to my PATH for some packages instead of the relevant python3.7.5 paths? System information
Describe the problemProvide the exact sequence of commands / steps that you executed before running into the problem.
At first I thought this was benoitc/gunicorn#1716, so I added Other info / logsSince there's no stacktrace back to mlflow, I next tried following the code base by eye to see how the error occurs. I'd like to note that
succeeds. Additionally, in a python shell, I can do the following without any issue:
However, if I copy
Next, look at the differences:
Now, let's figure out which is responsible:
So the issue is that
Huh, that's weird, I'm in python3.7.5, why are all these python3.7 paths here? Sure enough, if I try
Whereas in shell:
|
this is failing because gunicorn needs setuptools, not your package |
@jonringer doh! I tried the same fix earlier but wrote propogatedBuildInputs facepalm. Now I get a new error:
And yet this works:
|
the only thing i can think of, is that the new worker process isn't getting the same PYTHONPATH |
if mlflow is meant to be an "application", rather a package. You could move it out of python-modules and use |
@jonringer I need to use mlflow both as a program |
Had a useful conversation with @jtojnar on irc, so figured I'd copy here if only to refer back later:
I missed the long message at first, reposting here in case matrix deletes. I did an experiment that confirms @jtojnar's suspicions that PYTHONPATH is somehow not being set in the subprocess:
Output: https://gist.github.com/tbenst/f13ad655e6ad4cc6dae31c49b9f3643a A few things are very odd namely:
|
It appears that this issue is caused by #23676 |
What you're looking for is buildPythonApplication, I would recommend adding a
you just happened to construct the only path that matters, the python standard lib (site packages) gets put on PYTHONPATH due to the interpreter being wrapped:
|
while your package is in python-modules, you will need to patch the source code to allow for the packages to call commands or reference store paths. If you're just an application, then you're free to use wrapping mechanisms to your pleasure |
|
@jonringer ah, I didn't know about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personally, i would move mflow out of python-packages, and use buildPythonApplication
instead. It would solve most of your issues.
I am using Edit: no difference in behavior using here's the branch for buildPythonApplication: https://github.com/tbenst/nixpkgs/tree/mlflow-app |
@jonringer just wanted to check on this? I've been using this package successfully in production for over a week now with no issues. Let me know if you think any of above threads are not resolved |
3c913ac
to
c86f61a
Compare
@jonringer ok implemented #74091 (comment). I think this addresses final concern |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on NixOS. Functions as expected when used with --no-conda
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly LGTM
a little concerned about the non-standard mlflow-server package, but it is a convenient way to make a gunicorn web server. @FRidh thoughts?
@jonringer thanks for the excellent feedback and support on this issue! Learned a lot. This is ready for final review. @worldofpeace @disassembler would love to get this in 20.03 milestone as possible |
af9b214
to
9fe16c4
Compare
@tbenst Backports of adding packages is fine even after the release. |
Made the requested changes! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
[17 built, 2 copied (0.4 MiB), 0.1 MiB DL]
https://github.com/NixOS/nixpkgs/pull/74091
11 package built:
mlflow-server python27Packages.databricks-cli python27Packages.gorilla python37Packages.databricks-cli python37Packages.gorilla python37Packages.mlflow python37Packages.querystring_parser python38Packages.databricks-cli python38Packages.gorilla python38Packages.mlflow python38Packages.querystring_parser
@worldofpeace possible to add this to 20.03 / to 20.03 backports? Let me know how I can help. Thank you! |
Motivation for this change
add mlflow, an Open source platform for the machine learning lifecycle. Note that this package is only partially functional on NixOS, and is not intended to support features requiring conda.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)