PEP 694: Amend scanning in staged releases and add legacy API changes to use staged releases - #5070
PEP 694: Amend scanning in staged releases and add legacy API changes to use staged releases#5070cjames23 wants to merge 1 commit into
Conversation
…API changes to use staged releases
|
All commit authors signed the Contributor License Agreement. |
Documentation build overview
6 files changed ·
|
hugovk
changed the title
hugovk
changed the title
warsaw
left a comment
warsaw
left a comment
There was a problem hiding this comment.
This is a great addition to the PEP. I have some comments for a few things that need clarification, but otherwise +1. And welcome aboard as a co-author!
| so that (for example) a CI job can install-test a staged release without project upload credentials. | ||
|
|
||
| As one such stricter policy, an index **MAY** require *additional* authorization, beyond upload permission, to | ||
| :ref:`publish <publishing-session-completion>` or :ref:`cancel <publishing-session-cancellation>` a session, |
There was a problem hiding this comment.
+1 for publishing, but can you provide some additional scenarios about why cancelation should require additional authorization?
| **MAY** treat it as though it had completed without an adverse result and publish the session, so that a | ||
| backlogged or unavailable review system does not indefinitely prevent publication. If the review produces an | ||
| adverse result, the session resolves to ``error`` with the reason reported in the session's ``notices``; the | ||
| server **MAY** decline to publish such a session on any subsequent retry, in which case it is eventually |
There was a problem hiding this comment.
This section (if not covered below), should provide details on what happens if files in the session are deleted and reuploaded. It could be:
- Continue to await human review
- Rescan newly uploaded files
- Reset the entire malware scan state
It probably would also be helpful to say something about denial of service attacks on the scanner. E.g. bad actor starts an upload session, uploads known malware knowing it will get flagged. Bad Actor deletes the files and reuploads them to trigger another scan. Rinse and repeat until the scanning service is overwhelmed.
|
|
||
| .. _legacy-interop: | ||
|
|
||
| Legacy Upload API Interoperability |
There was a problem hiding this comment.
I wonder if it makes sense to be more explicit about the "control plane" API for managing the session (and file uploads), and/or lump this and the explicit session creation together? Maybe add a top-level "Session Creation" section, putting this and the original session creation request together, then the rest of it would be the same control plane API.
It also occurs to me that we might want to update the state diagrams so that it's obvious that there are two entry points "open" state. However, if I'm reading this section correctly, the index can also auto-publish if the conditions allow (no adverse scan or scan timeout). Maybe that'll make the diagram too cluttered in which case we could skip it or add a second diagram. LMK what you think and I can take a crack at updating the diagram once this PR lands.
The other thing this section should discuss is the interaction between legacy and 2.0 sessions. E.g. what happens if there's an open 2.0 session? Can someone use the legacy API to add a file to that existing session? I would think that would get rejected with a 409. Also vice versa (meaning: do we allow mixing of session creation requests?). We just need to be explicit about how these mix.
2 participants
Amendments based on DPO discussion https://discuss.python.org/t/pre-pep-staged-releases-separated-from-pep-694/107804/59
@warsaw - here are my proprosed amendments which I would like your sign off on as well.