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

Add option for wheelhouse overrides #338

Merged
merged 2 commits into from Aug 25, 2017
Merged

Conversation

johnsca
Copy link
Contributor

@johnsca johnsca commented Aug 24, 2017

To facilitate CI and testing of charms with new versions of Python dependencies, particularly those from base layers, you can now provide a wheelhouse.txt file via the charm-build CLI that will be applied
after all others which can thus override any dependencies from lower layers.

To facilitate CI and testing of charms with new versions of Python
dependencies, particularly those from base layers, you can now provide
a wheelhouse.txt file via the `charm-build` CLI that will be applied
after all others which can thus override any dependencies from lower
layers.
@johnsca
Copy link
Contributor Author

johnsca commented Aug 24, 2017

@ryan-beisner

Copy link
Contributor

@marcoceppi marcoceppi left a comment

Choose a reason for hiding this comment

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

Would like to see tests, but :LGTM: otherwise.

next_config,
)
if existing_tactic is not None:
output_files['wheelhouse.txt'].combine(existing_tactic)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we combine or just be an overwrite?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind, thinking about it combine makes more sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Combine merges the files. In fact, we end up with both versions of the files in the wheelhouse, which is also something that can happen with multiple layers referencing the same dep. It would be great to be able to resolve that up front, but that would require reimplementing much of pip's dependency resolution, so I'm disinclined to even try. It does generally do the right thing at the end, but it may be something we need to address eventually.

@johnsca
Copy link
Contributor Author

johnsca commented Aug 24, 2017

@marcoceppi I can look at adding / extending tests tomorrow.

@marcoceppi
Copy link
Contributor

Thanks @johnsca, looks great! Should be in edge shortly.

@marcoceppi marcoceppi merged commit 5761a72 into master Aug 25, 2017
@johnsca johnsca deleted the feature/wheelhouse-override branch September 18, 2019 07:28
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

2 participants