-
Notifications
You must be signed in to change notification settings - Fork 910
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
Fix inconsistent requirements handling in kedro pipeline package
vs. kedro package
#1536
Comments
Originally posted by @noklam in #2761 (review) |
One more comment |
We decided in #3750 to deprecate |
Separated from an old ticket #1077.
Description
We're running into some problems with "unparseable" requirements in requirements.txt. By this I mean requirements which are valid in a requirements file (i.e. when you do
pip install -r requirements.txt
) but are not valid ininstall_requires
in setup.py. These are requirements like:The common cause of bug reports (KED-2897, KED-2346) is the local whl file case, but the other things above will cause exactly the same sort of error. In the case of a local wheel file there is a workaround (use
package @ file:///path/to/package.whl
) but in the other cases there isn't. Packaging a local wheel or the other options doesn't make sense anyway, so the error message is fair enough.Now there's currently three places where this is relevant:
kedro pipeline package
. Following https://github.com/quantumblacklabs/private-kedro/pull/1280, this gives an error when a user tries packaging a pipeline with unparseable requirements in requirements.txt.kedro package
. Currently this gives an error for some and silently ignores other unparseable requirements.kedro build-docs
. This crashes horribly when there's unparseable requirements in requirements.txt (KED-2346), which shouldn't be the case - a user should be able to build docs regardless of what's in their requirements file. Let's forget about this case for the purposes of this issue; will be dealt with separately.Problem
The way requirements are handled is inconsistent between
kedro pipeline package
andkedro package
(and arguably even withinkedro package
).Solution
Either make both commands silently ignore unparseable requirements or make both commands fail as soon as there's any unparseable requirement. I can see arguments both ways here but prefer the hard fail option - as @lorenabalan said, otherwise "it gives the wrong impression to the user that the resulting package can actually be run somewhere else, when in reality it can't".
@idanov has a slight preference for ignoring unparseable requirements but emitting a warning message saying that they're being ignored. But this isn't a strong opinion.
The text was updated successfully, but these errors were encountered: