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

Serve run_exports.json in conda channels #51

Merged
merged 9 commits into from
Aug 8, 2023

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented May 4, 2023

@jaimergp
Copy link
Contributor Author

jaimergp commented Jun 2, 2023

Hi @conda-incubator/steering, any thoughts or feedback here? Thanks 🙏

@wolfv
Copy link
Contributor

wolfv commented Jun 2, 2023

I think this is a great idea and will also simplify rattler-build!

I am leaning towards adding an empty run exports for all packages (instead of leaving out those that don't have it) so that one knows that these were considered.

Copy link

@beckermr beckermr left a comment

Choose a reason for hiding this comment

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

We should state explicitly if we allow patching and if the values in this file taken precedence over the ones in the artifacts.

@wolfv
Copy link
Contributor

wolfv commented Jun 3, 2023

good point @beckermr! There is also pin_as_run in the conda_build_config.yaml file. I believe that that one actually takes precedence over any run exports. Might be good to mention that in the CEP as well.

@jaimergp
Copy link
Contributor Author

We should state explicitly if we allow patching and if the values in this file taken precedence over the ones in the artifacts.

Right now conda-build (and I would imagine other implementations) have not been notified of run_exports.json existence, so I'd say that initially it won't have an impact on building packages. The file is there as a convenience for automation infrastructure. Taken that into account, I'd be inclined to say that the file is not going to be patched for now.

If the community decides that's convenient, then we can create another (smaller) CEP to change that. I think I have read feedback from some users (cannot recall who, sorry) who were against run_exports patching.

@jaimergp
Copy link
Contributor Author

There is also pin_as_run in the conda_build_config.yaml file

TIL! I'll check how it works and add it to the CEP. Thanks Wolf!

@jaimergp
Copy link
Contributor Author

jaimergp commented Jun 16, 2023

I am leaning towards adding an empty run exports for all packages (instead of leaving out those that don't have it) so that one knows that these were considered.

@wolfv, my thinking here is that the source of truth for all available packages is still repodata.json. run_exports.json simply extends that with extra fields for some. If we enumerate all packages in run_exports.json then we also have to account for removals with "marked as broken" and so on. Does that sound reasonable?

That said, we could always ignore the differences between both. Listing all files might incur some extra download size but will for sure list everything that was analysed, which should make debugging easier...

The more I think about it, the more partial I am to your proposal 😬

@jaimergp
Copy link
Contributor Author

Added my current view regarding precedence and patching to the CEP. Happy to revisit, but for now I think the easiest is to consider run_exports.json a mirror for the info already available in the archives. This means it should not be patched or given a special treatment wrt pinning resolution in conda-build tooling.

cc @beckermr @wolfv

@jaimergp
Copy link
Contributor Author

jaimergp commented Jul 6, 2023

For the record, I plan to call a vote for this in two weeks (next conda community call), so if you have any comments before the vote, this is the time. Thanks! 🙏

cep-9999.md Outdated Show resolved Hide resolved
cep-9999.md Outdated Show resolved Hide resolved
cep-9999.md Outdated Show resolved Hide resolved
cep-9999.md Outdated Show resolved Hide resolved
@chenghlee
Copy link

Suggested some minor tweaks, but I am in favor of this proposal.

@jaimergp
Copy link
Contributor Author

Thank you @chenghlee! Added your points in the new commits.

@jaimergp
Copy link
Contributor Author

@conda-incubator/steering, I would like to call a vote for this CEP.

This vote falls under the "Enhancement Proposal Approval" policy of the conda governance policy,
please vote and/or comment on this proposal at your earliest convenience.

It needs 60% of the Steering Council to vote yes to pass.

To vote, please leave yes, no or abstain as comments below.

If you have questions concerning the proposal, you may also leave a comment or code review.

This vote will end on August 3rd, 2023 End of Day, Anywhere on Earth (AoE).

@jezdez
Copy link
Member

jezdez commented Jul 18, 2023

yes

@chenghlee
Copy link

Yes

@wolfv
Copy link
Contributor

wolfv commented Jul 19, 2023

yes

@marcelotrevisani
Copy link
Member

yes!

Copy link
Member

@mbargull mbargull left a comment

Choose a reason for hiding this comment

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

Yes!

@jezdez
Copy link
Member

jezdez commented Jul 20, 2023

This vote will end on August 3rd, 2023 End of Day, Anywhere on Earth (AoE).

@jaimergp Any reason to expand the vote from 7 to 16 days BTW?

@jaimergp
Copy link
Contributor Author

@jezdez To be able to invoke the timeout quorum rules and have two in-call reminders 😁

Copy link

@xhochy xhochy left a comment

Choose a reason for hiding this comment

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

Yes!

cep-9999.md Outdated Show resolved Hide resolved
Co-authored-by: msarahan <msarahan@users.noreply.github.com>
@jaimergp
Copy link
Contributor Author

Yes.

Copy link
Contributor

@msarahan msarahan left a comment

Choose a reason for hiding this comment

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

Yes

Copy link
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

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

Yes please!

@jaimergp
Copy link
Contributor Author

jaimergp commented Aug 7, 2023

Hello team! The vote concluded last week on Aug 3rd at 23:59 AOE (Aug 4th 11:59 UTC). I have now reviewed the votes and these are the results:

  • Yes: 12
  • No: 0
  • Abstain: 0
  • Available quorum: 14
  • Criteria: 60% of quorum
  • 12/14 = 85.7%
  • ✅ The vote has passed!

--

I will now update the PR to reflect the status and mint a CEP number. Since the vote for this PR was started after #8, this CEP will be minted the number 11.

@jaimergp
Copy link
Contributor Author

jaimergp commented Aug 7, 2023

Sorry, small mistake, this CEP will get number 12, not 11.

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