conda / ceps

Conda Enhancement Proposals
Creative Commons Zero v1.0 Universal
19 stars 24 forks source link

CEP 6: Add Channel Notifications to conda #19

Closed travishathaway closed 2 years ago

travishathaway commented 2 years ago

This pull request contains a draft CEP for the implementation of a new "Channel Notifications" feature to the conda package manager. Please see the CEP itself for more information about the proposal.

Please use this pull request as a public forum for discussing the feature proposal as well as suggesting improvements.

wolfv commented 2 years ago

In generally I think it could be interesting for channels to be able to contain more metadata.

For example, if the robostack channel could easily advertise that it relies on the conda-forge channel to work, that could be cool. A channel metadata could further have some fields for description etc.

seibert commented 2 years ago

I think another useful thing to specify in this CEP is the expectation that the client should fetch and show notifications before fetching or parsing the repodata.json file. The reason for this is that if the repository is corrupted in some way, it would be nice for admins to quickly put up a notifications.json file explaining why and where to get more information while it is being fixed. Could be similarly useful if a backward incompatible change to the repo format ever needs to be made. A clever web server configuration could serve a special notifications.json file to clients before a certain version in order to notify the user of the compatibility issue.

LtDan33 commented 2 years ago
  1. What is the thought on priority for channels? In theory if you are pulling from many channels you could have quite a few. Is there any limit to the number of alerts?
  2. It was also pointed out that the “expiry” naming could perhaps be a little clearer.
  3. Also I think I like the idea of having a dedicated notification json, do we want any qualifiers for specific client/conda metadata? For ex:

There are always “what about…” so I’m all for keeping it simple and then expanding, but trying to think through other common and straightforward use cases that currently would add a lot of value. There are some implementation details on if there would be any parsing of the user agent to only return the “correct” alerts.json to the client, or if the server would throw back everything and let the client figure out what it wants to show.

travishathaway commented 2 years ago

@LtDan33 you bring up a good point about the number of messages being shown. When writing this I assumed that people may have about 4 or 5 channels in active use (e.g. defaults, conda-forge, bioconda, internal-corporate-one, internal-corporate-two). With ten or more, these messages could get pretty unwieldy.

To remedy this, I suggest we rely on channel priority like you suggested. For example, we could only display the top five channel notifications and instruct our users to read the full output by running conda notices.

I think the arch and conda_version fields would be a great addition to the message JSON Schema definition. These will be clearly marked as optional though.

kenodegard commented 2 years ago

I suggest we rely on channel priority like you suggested. For example, we could only display the top five channel notifications and instruct our users to read the full output by running conda notices.

Let's assume a user has 5 channels defined (A-E). Channel A defines 2 critical notices and one info notice, channel B/C both define one info notice, and D/E both define one critical notice. What notices do we prioritize if we show the top 5 notices?

  1. Display the most severe (and oldest) notice for the first 5 channels. Only one notice per channel.
    • A shows one critical notice
    • B shows one info notice
    • C shows one info notice
    • D shows one critical notice
    • E shows one critical notice
  2. Sort all notices by severity followed by channel priority. Display the top 5 notices.
    • A shows two critical notice
    • D shows one critical notice
    • E shows one critical notice
    • B shows one info notice

I don't think either of these is better than the other so it's probably best to only display one notice? That way there's no confusion as to what notice would be displayed. We could just include a statement that there are additional notices, and to run conda notices to see all of these notices.

travishathaway commented 2 years ago

@kenodegard You bring up an interesting point. I would err on the side of sorting by "priority", "channel order". We can play around with this in the initial implementation too.

In any case, we should definitely show a message instructing users if there are more unread messages. Something like:

5 of 12 new messages run `conda notices` to view all
LtDan33 commented 2 years ago

@kenodegard Good points. @travishathaway I like the thinking around making it explicit that there is value and a reason to run the command. Wordsmithing aside, I think we should make sure to call out any critical notices that aren't shown. Something like "X notices not shown, including y critical."

I'd also make sure we are consistent with the naming throughout around messages/notices/etc and just use one everywhere. Ubiquitous language ftw 😄

travishathaway commented 2 years ago

Hi all,

This is a recent video I have made demonstrating what this feature could look like once it is ready:

https://user-images.githubusercontent.com/718726/168361745-73adb527-8a13-4553-a906-1a51ae69b070.mp4

Looking forward to comments. If you have code specific comments, please take it over to the open PR I have current have for this: https://github.com/conda/conda/pull/11462

jezdez commented 2 years ago

@conda-incubator/steering

This PR falls under the Enhancement Proposal Approval policy of the conda governance policy, please vote and/or comment on this PR.

This PR needs 60% of the Steering Council to vote yea to pass.

To vote please leave Approve (yea) or Request Changes (nay) pull request reviews.

If you would like changes to the current language please leave a comment (in the PR) or push to this branch.

This vote will end on 2022-06-15.

jezdez commented 2 years ago

Hi @conda-incubator/steering team, please make sure to review and vote on this CEP, thank you!

travishathaway commented 2 years ago

There were recent changes to this CEP. We have dropped the expiry field on individual message in exchange for an expired_at field. This new field will be set as an ISO 8601 timestamp string just like created_at.

jezdez commented 2 years ago

Voting results

This was a standard, non-timed-out vote.

This vote has reached quorum (15 + 0 = 15 which is at least 60% of 21).

It has also passed since it recorded 15 "yes" votes and 0 "no" votes giving 15/15 which is greater than 60% of 15.

It should be noted that multiple requests for change were recorded in the pull request about minor implementation details that do not invalidate the previous votes. The author @travishathaway made the requested change.