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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load micropackage dependencies statically #2989

Closed
wants to merge 1 commit into from

Conversation

astrojuanlu
Copy link
Member

@astrojuanlu astrojuanlu commented Aug 31, 2023

NOTE: Kedro datasets are moving from kedro.extras.datasets to a separate kedro-datasets package in
kedro-plugins repository. Any changes to the dataset implementations
should be done by opening a pull request in that repository.

Description

This avoids building the wheel in an isolated environment just to extract the dependencies.

Cons: It relies on a PEP-621 compliant pyproject.toml file to be present, which only happens for Kedro projects created after #2280 was closed. Therefore, it is backwards incompatible.

Pros: It makes kedro micropkg pull fast again, cutting the time to run the full test suite in ~half (from 12 to 7 minutes on Linux, from 14 to 6 minutes on Windows 馃帀)

Development notes

This uses pyproject-metadata instead of PyPA/build, see https://pypi.org/project/pyproject-metadata/

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes
  • Checked if this change will affect Kedro-Viz, and if so, communicated that with the Viz team

@astrojuanlu astrojuanlu force-pushed the accelerate-micropkg branch 4 times, most recently from 2df789a to 1932817 Compare September 1, 2023 17:29
@astrojuanlu astrojuanlu changed the base branch from main to develop October 23, 2023 23:01
@astrojuanlu astrojuanlu force-pushed the accelerate-micropkg branch 3 times, most recently from 0f9bf7d to c574116 Compare October 24, 2023 18:21
@noklam
Copy link
Contributor

noklam commented Oct 24, 2023

Is it possible to keep both, using the Metadata when available and fallback if not?

@astrojuanlu
Copy link
Member Author

Yes indeed!

@astrojuanlu
Copy link
Member Author

I don't have time to pursue the backwards-compatible approach, leaving this as a POC in case anybody wants to pick it up. Essentially this would need

  • To retain build as a dependency,
  • Conditionally try to locate the PEP-621 metadata in pyproject.toml and otherwise use the slow codepath, and
  • Test both things.

Signed-off-by: Juan Luis Cano Rodr铆guez <juan_luis_cano@mckinsey.com>
@merelcht
Copy link
Member

@astrojuanlu Would it make sense to write this up in a ticket instead of keeping a PR open that will become more and more outdated? It's not super likely that this will be merged as is since it's from August and solving conflicts might be a real pain.

@astrojuanlu
Copy link
Member Author

Good idea @merelcht , will do!

@astrojuanlu
Copy link
Member Author

Closing in favour of #3457

@astrojuanlu astrojuanlu deleted the accelerate-micropkg branch December 21, 2023 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants