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

Build wheel with pip if build is not available #323

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MortGron
Copy link
Contributor

@MortGron MortGron commented Oct 15, 2024

Description

Make the build wheel function work even if the user forgot to install the 'cli' dependencies. The only assumption is that the user has a reasonably new version of pip installed, which will be the case almost always.

Another solution could be to include 'build' as part of the non-optional dependencies.

Checklist:

  • Tests added/updated.
  • Documentation updated.
  • Changelog updated in CHANGELOG.md.
  • Version bumped. If triggering a new release is desired (?), bump the version number by running python dev.py bump --patch (replace --patch with --minor or --major per semantic versioning).
  • Regenerate example SDKs export PYTHONPATH=. && python dev.py generate. Need to be run both
    for pydantic v1 and v2 environments.

Copy link

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
3693 2588 70% 60% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
cognite/pygen/_build.py 36% 🟢
TOTAL 36% 🟢

updated for commit: 84db6e3 by action🐍

@MortGron
Copy link
Contributor Author

@doctrino what do you think of this?

Meant as an idea. If you agree I can extend to a complete PR.

if "build" in sys.modules:
ProjectBuilder(build_dir).build(distribution="wheel", output_directory=str(output_dir))
else:
subprocess.run([sys.executable, '-m', 'pip', 'wheel', '--no-deps', '--no-build-isolation', '-w', output_dir, build_dir], check=True)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--no-build-isolation wil make this work even when the user has no internet connection

@doctrino
Copy link
Collaborator

doctrino commented Oct 19, 2024

@doctrino what do you think of this?

Meant as an idea. If you agree I can extend to a complete PR.

Hmm, a few questions: Is there a particular use case for it? Or more a slight convenience to not have to run pip install ...? Anyone asked for it?

It is not necessary, I am happy to approve it either way, but it is always good to ensure we are not adding more code just in case as maintenance is always an issue.

@MortGron
Copy link
Contributor Author

@doctrino what do you think of this?
Meant as an idea. If you agree I can extend to a complete PR.

Hmm, a few questions: Is there a particular use case for it? Or more a slight convenience to not have to run pip install ...? Anyone asked for it?

It is not necessary, I am happy to approve it either way, but it is always good to ensure we are not adding more code just in case as maintenance is always an issue.

I have first-hand experience with CDF being used in enterprise environments where PyPi is blocked (together with much of the internet), and where there might be some approval process for the packages you want to install or some other unconventient process to get new Python packages into the enterprise environment.

@MortGron
Copy link
Contributor Author

The difference between the options as I see it are:

  • Making "build" non-optional: minimal change from how it currently works.
  • Building wheel with "pip": could eventually drop "build" without adding almost any code. Would have one less dependency to take care off.

@doctrino
Copy link
Collaborator

@doctrino what do you think of this?
Meant as an idea. If you agree I can extend to a complete PR.

Hmm, a few questions: Is there a particular use case for it? Or more a slight convenience to not have to run pip install ...? Anyone asked for it?
It is not necessary, I am happy to approve it either way, but it is always good to ensure we are not adding more code just in case as maintenance is always an issue.

I have first-hand experience with CDF being used in enterprise environments where PyPi is blocked (together with much of the internet), and where there might be some approval process for the packages you want to install or some other unconventient process to get new Python packages into the enterprise environment.

Got it, that is a good use case, go for it.

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.

2 participants