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

Fix inconsistent requirements handling in kedro pipeline package vs. kedro package #1536

Closed
antonymilne opened this issue May 16, 2022 · 3 comments

Comments

@antonymilne
Copy link
Contributor

antonymilne commented May 16, 2022

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 in install_requires in setup.py. These are requirements like:

--extra-index-url https://this.wont.work
-r other_requirements.txt
./path/to/package.whl
http://some.website.com/package.whl

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:

Problem

The way requirements are handled is inconsistent between kedro pipeline package and kedro package (and arguably even within kedro 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.

@astrojuanlu
Copy link
Member

First, I expect kedro pipeline create has symmetry to kedro micropkg package. i.e. kedro pipeline create nok then kedro micropkg package nok should work, but it doesn't because I need to do kedro micropkg pipelines.nok which isn't trivial to find in the doc. I end up have to look at the e2e test. The documentation doesn't tell me I need to do this https://docs.kedro.org/en/latest/nodes_and_pipelines/micro_packaging.html

Originally posted by @noklam in #2761 (review)

@noklam
Copy link
Contributor

noklam commented Aug 3, 2023

One more comment
kedro package nok -> python -m nok would work and this is what @idanov suggest as a general way to deploy kedro
kedro micropkg package nok -> python -m nok would not work because it doesn't have the __main__.py .

@astrojuanlu
Copy link
Member

We decided in #3750 to deprecate kedro micropkg so we will not do this.

@astrojuanlu astrojuanlu closed this as not planned Won't fix, can't repro, duplicate, stale May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

4 participants