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

Feature request: Option to disable no-binary pip option for non-production builds #1115

Closed
carlcsaposs-canonical opened this issue May 31, 2023 · 12 comments
Labels
Enhancement New feature or request triaged

Comments

@carlcsaposs-canonical
Copy link
Contributor

What needs to get done

Add charmcraft pack option to disable --no-binary pip option

cmd = [pip_cmd, "install", "--upgrade", "--no-binary", ":all:"] # base command
for non-production builds

Why it needs to get done

Installing Python packages from source can take up to an hour for charms with a lot of dependencies

Moving the dependencies from requirements.txt to charm-binary-python-packages decreases charmcraft pack time to a couple of minutes

For production builds, installing Python packages from source is desirable. However, for non-production development builds, this can significantly slow down iteration speed.

@carlcsaposs-canonical carlcsaposs-canonical changed the title Option to disable no-binary pip option for non-production builds Feature request: Option to disable no-binary pip option for non-production builds May 31, 2023
@lengau lengau added Enhancement New feature or request triaged labels Jul 6, 2023
@lengau
Copy link
Collaborator

lengau commented Jul 6, 2023

Can you give more details about your workflow? I can see the first build taking a long time, but for iteration even with --no-binary it should still be using cached wheels. If it's not that's probably a bug (in addition to this feature request).

@carlcsaposs-canonical
Copy link
Contributor Author

This is for first builds—on CI or new VMs or after updating something that requires a charmcraft clean (e.g. requirements.txt change, actions.yaml change, charmcraft.yaml change)

@lengau
Copy link
Collaborator

lengau commented Jul 25, 2023

Quick note while I'm thinking about this: the resulting charm should probably get a flag that makes it un-publishable in the store (or at very minimum not to a stable track — maybe similar to snapcraft's grade).

@sed-i
Copy link
Contributor

sed-i commented Aug 10, 2023

I can see the first build taking a long time, but for iteration even with --no-binary it should still be using cached wheels.

Apart from CI, this is also a pain when using a VM for charm development.
VMs are somewhat disposable so the "first build" occurs more often than one would think.

There is a growing interest in (ab)using charm-binary-python-packages.

the resulting charm should probably get a flag that makes it un-publishable in the store

Why not? What's the difference between a charm packed in those two different ways?

@dstathis
Copy link
Contributor

dstathis commented Dec 5, 2023

Any plans to do this? It would really streamline a development a lot.

Also, I tend to see charm-binary-python-packages in most charm repos these days. This is presumably not what we want and I think people would stop if they could build more quickly.

@lengau
Copy link
Collaborator

lengau commented Dec 7, 2023

This isn't on the current cycle roadmap. However, the new cross-container caching should help alleviate this a bit. On Linux, this cache is stored in ~/snap/charmcraft/common/cache/. Please continue to vote on this issue to help show support for this feature.

@jnsgruk
Copy link
Member

jnsgruk commented Dec 7, 2023

This isn't something we should implement, and I think this issue should be closed.

The global cache feature that Alex mentioned should really help a lot here, and should get build times down to the same as if you were consuming a binary wheel from PyPI after the first run.

@sergiusens / @lengau we should add an entry to the Charmcraft docs on juju.is that explains how to wire the cache up to Github Actions if we haven't already :)

@lengau
Copy link
Collaborator

lengau commented Dec 8, 2023

Per Jon's comment above I'm marking this as not planned. We've got a task for the documentation, and @carlcsaposs-canonical has an excellent workshop on improving CI for charmcraft.

Notwithstanding bugs in the caching process, I believe the feature can help with most of the underlying situations that make this feature desirable. (Mounting that cache directory from a non-ephemeral location should work for CI and virtual machines.)

@lengau lengau closed this as not planned Won't fix, can't repro, duplicate, stale Dec 8, 2023
@carlcsaposs-canonical
Copy link
Contributor Author

charmcraftcache helps alleviate this issue

(and simplifies "how to wire the cache up to Github Actions" [while also speeding up non-CI builds])

@ca-scribner
Copy link

I am sympathetic to why we require binaries be built locally for a charm that is being published, but the approach we're taking to enforce this is not working. This is visible from how several teams have implemented workarounds to reduce their CI build times (some use charm-binary-python-packages, others use things like @carlcsaposs-canonical mentioned). By not providing a good solution, people are making their own.

A charmcraft argument for this would make it easy to:

  • use existing binaries for ephemeral builds (local and iterations in CI)
  • build your own binaries specifically when publishing to charmhub

charmcraft.yaml's charm-binary-python-packages makes is possible for charmers to pull binary packages, but since it is in a config file it is awkward to enable/disable. A charmcraft flag is better suited to this.

If we're worried about people abusing this feature and publishing charms with binaries they did not build, have charmcraft write metadata that charmhub inspects to reject charms with public binaries

Copy link

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/CRAFT-2866.

This message was autogenerated

@carlcsaposs-canonical
Copy link
Contributor Author

others use things like @carlcsaposs-canonical mentioned

to clarify, all data platform repos & repos using charmcraftcache (to my knowledge) respect the requirement you mentioned:

we require binaries be built locally for a charm that is being published

(we use charmcraftcache for integration test builds, but not for release builds)

this does have the unfortunate side effect that the charm we release is not the charm that we test with

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request triaged
Projects
None yet
Development

No branches or pull requests

6 participants