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

[RFC] Python packages for helm binaries #631

Closed

Conversation

chrisglass
Copy link

@chrisglass chrisglass commented Oct 22, 2020

This pull requests proposes to move the helm bindings for kapitan to their own (python) package, currently available here.

While this branch and the related repository do work today, some polish would still be needed before a full release. In particular, documentation and tests have not been updated yet.

Please use this pull request to discuss the approach taken, keeping in mind that it is not yet "merge ready". I will fix the tests and documentation etc... once the approach is validated.

Caveats: for this proof of concept only linux is supported, but work to publish macos binaries is planned.

Testing this PR manually

Checkout this branch, then in a pristine virtualenv:

pip install -r requirements.txt
pip install -e .  # This installs kapitan from the current branch
pip install --index-url https://test.pypi.org/simple/ kapitan_helm  # Notice: this is *test* pypi
kapitan compile ...

This should allow you to use helm charts as inputs.

Running automated tests

From a pristine virualenv

pip install -r requirements.txt
make test

Instead of assuming the bindings will be built locally, rely on
importing pre-built binairies shipped as the kapitan_helm package.
This commit removes the code that is now build and distributed by
kapitan_helm (the golang shim and python CFFI machinery).
@google-cla
Copy link

google-cla bot commented Oct 22, 2020

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Oct 22, 2020
@chrisglass
Copy link
Author

@googlebot I signed it!

@google-cla google-cla bot added cla: yes and removed cla: no labels Oct 23, 2020
Instead of building the dependency manager for hlm charts as another
golang module (using CFFI), rely on the kapitan_helm distributing the
prebuilt binaries instead.
This commit should fix the build, CI pipelines and tests by declaring
the helm bindings as a dependency (for now, pointing to test pypi).
The pyinstaller binary builder expects some .so objcts to exist in the
tree, but that should be moved (and bundled) by dependencies instead.
@ramaro
Copy link
Member

ramaro commented Oct 25, 2020

Hey @chrisglass thanks very for this! I think it makes perfect sense to make it a binary, separate dependency. Would you mind if we fork your kapitan-helm-bindings repo under kapicorp to make an official pypi package?

@chrisglass
Copy link
Author

@ramaro I don't mind at all, on the contrary I think that's the right approach. You should also publish packages to pypi ASAP in my humble opinion (at least to claim the name, if it is suitable).

Building the binaries from that repository should be straightforward, hopefully, but don't hesitate to reach out if you need to.

@chrisglass
Copy link
Author

The last failing test seems to be related to the binary installer (it assumes built binaries). I'm no sure if the binary installer really has a reason to exist should this PR land - but if it does, then I will need guidance on how to solve the problem with the tests (I'm not really sure what pyinstaller is doing).

@ramaro ramaro mentioned this pull request Nov 26, 2020
@ramaro
Copy link
Member

ramaro commented Dec 30, 2020

@chrisglass regarding the binary, have you trying adding kapitan_helm as a --hidden-import, like in https://github.com/deepmind/kapitan/blob/master/scripts/pyinstaller.sh#L19 ?

As suggested on the pull request.
@chrisglass
Copy link
Author

That doesn't seem to help unfortunately.

I guess I'll try and spend some time during the end of year to dig further into the failure.

@ramaro
Copy link
Member

ramaro commented Aug 1, 2021

closing in favor or #701

@ramaro ramaro closed this Aug 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants