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

PEP 002 - Mixin Versioning #7

Closed
wants to merge 3 commits into from

Conversation

carolynvs
Copy link
Member

Bundles should be able to specify not only the mixin used, but its version and source. Porter should use that information to automatically download the mixins declared by the bundle.

New PEP Checklist

  • Is your pull request named: PEP NNN - <TITLE>?
  • Have you posted your proposal in the forum?
  • Have you gotten approval and a PEP number from a maintainer?
  • Did you use the PEP Template?

Forum Suggestion Link

This feature was originally suggested at getporter/porter#975 and by @sestegra in getporter/porter#1311

Bundles should be able to specify not only the mixin used, but its version and source.
Porter should use that information to automatically download the mixins declared by the bundle.

Signed-off-by: Carolyn Van Slyck <[email protected]>
@carolynvs carolynvs marked this pull request as draft February 21, 2021 19:10

#### porter mixins uninstall

❓ I am not sure if it makes sense to have keep this command? At the bundle level, removing a mixin doesn't seem to have a use case. They could just stop using it. However at the global cache level, what would really be useful is to clear the cache, or removing infrequently used mixin versions. I think cache management like that should be out-of-scope.
Copy link

Choose a reason for hiding this comment

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

This sounds like the right behavior to have here. Having mixin management at the bundle level makes it a little confusing as to what's happening. +1 to cache management seems out of scope here.

Porter really needs to manage the mixin cache and appropriately communicating with the correct mixin version targeted by the bundle.

* Mixin authors are impacted by this change because they will need to adjust their mixin declaration schema to allow for declaring the mixin version.
* Bundle authors are impacted by the new syntax for declaring mixins and no longer managing the bundle cache by hand.
Copy link

Choose a reason for hiding this comment

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

I'm navigating parsing the impact here and I'm getting a little confused.

Is it something like Bundle authors are impacted as versioning for mixins will change leading to more usability to be explicit with versioning of mixings but with the new syntax if no version information is specified latest will be used and bundle authors will no longer be able to manage the bundle cache by hand?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now if I am writing a bundle that needs v1.2.3 of a mixin I have to be aware of which version of that mixin is installed on the computer that is building the bundle. I am seeing authors script out, porter mixins install MIXIN --version v1.2.3 for each mixin they use, following by a porter build. This is the manual cache management that I am referring to.

With this change, they do not need to write extra scripts around porter build and porter will handle that for them. So bundle authors need to start setting a version for each mixin that their bundles use, and stop running extra scripts that they may have written.

pep/002-mixin-versioning.md Outdated Show resolved Hide resolved
pep/002-mixin-versioning.md Outdated Show resolved Hide resolved
Signed-off-by: Carolyn Van Slyck <[email protected]>
pep/002-mixin-versioning.md Show resolved Hide resolved
pep/002-mixin-versioning.md Show resolved Hide resolved

* The mixin name and url must match.
* The resolved version must match the version range defined in porter.yaml

Copy link

Choose a reason for hiding this comment

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

Generate Merkle Tree of the digests of all the mixins with the bundle as a root, where a Merkle Tree is a hash tree that provides an efficient way to verify the contents or mixins of a set data are present and unchanged. if one of the leaf's mixins digest changes, the Merkle Tree is recalculated. that way we can for sure determine that the bundle changed and we can act on that. Therefore, the lockfile is invalidated
REF:

Copy link

@MChorfa MChorfa Feb 26, 2021

Choose a reason for hiding this comment

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

lol I had this comment on my tab 2 days ago and forgot the submit it 😅

@carolynvs
Copy link
Member Author

I think we should put a hold on this because if we implement mixins as bundles, it saves a lot of work. There is a lot of good info here that is still relevant, but we could avoid having to implement a cache. I'd like to pause and revisit this after we make a decision on the other proposal.

@carolynvs
Copy link
Member Author

This is on hold because the solution to a lot of the problems posed in this PEP are solved by getporter/porter#1499

@carolynvs
Copy link
Member Author

Closing since we have accepted #11 which will solve a lot of the same problems. The lockfile is still needed and I'll copy that over into the specification for that PEP so we don't lose track of it.

@carolynvs carolynvs closed this Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants