Repository navigation
feat: create git repo for docs - #1561
Conversation
|
TODO before deploy: set WRITE token |
jennifer-richards
left a comment
jennifer-richards
left a comment
There was a problem hiding this comment.
I think it'd be worth at least pulling some consts from settings, rephrasing the notification slightly, and probably extracting a reusable helper (if we don't have one already). See inline.
A couple of other comments are in there also, mostly as questions. I don't think changing the ARCHIVE_URL is necessary without more thought and refactoring notification lengths should be done separately, but I'm leaving the comments.
| TEMPLATE_REPO = "base-template" | ||
| BRANCH = "Approved" | ||
| EDITS_BRANCH = "RPC-edits" | ||
| COMMIT_MESSAGE = "approved I-D" | ||
| ARCHIVE_URL = "https://www.ietf.org/archive/id/" | ||
| REQUEST_TIMEOUT = 30 # seconds | ||
| # GitHub populates a repository created from a template asynchronously. Until it | ||
| # has done so, requests for the branch return 409 ("Git Repository is empty") or | ||
| # 404. | ||
| BRANCH_WAIT_ATTEMPTS = 10 | ||
| BRANCH_WAIT_SECONDS = 2 | ||
| _NOT_READY_STATUSES = (404, 409) |
There was a problem hiding this comment.
Some of these seem like they should perhaps be configurable via settings. TEMPLATE_REPO, ARCHIVE_URL, and the timeouts/wait parameters seem like candidates.
Speaking of ARCHIVE_URL, I wonder if we can give purple more direct access to the files than by going out through CF. Not sure, because reading from R2 may be slower and I don't think we want to give purple direct access to blobdb.
There was a problem hiding this comment.
I think we could add a DT API endpoint to return those file contents... but that could be handled outside the PR
| hint = " (needs manual fix)" if isinstance(err, DraftRepoExists) else "" | ||
| Notification.emit( | ||
| Notification.EventType.REPO_NOT_CREATED, | ||
| f"No repo created for {rfctobe.name}: {err}{hint}"[:255], |
There was a problem hiding this comment.
I think the length limit on a notification is somewhat arbitrary and it might be worth extending and putting the UI in charge of ellipsizing if necessary. Note that rfctobe.name can be 255 characters on its own, and err can potentially have long content due to the filename (likely including the name again). There's no guarantee that err / hint show up in the first 255 characters.
An alternative that would address what first caught my eye is to move the hint earlier, so that a hinted notification would look like "No repo created (needs manual fix) for draft-ietf-this-has-a-really-long-name-to-be-a-nuisan..." and the known-length parts would be guaranteed visible.
| ) | ||
|
|
||
|
|
||
| def _rpc_person_of(user_id: int | None) -> RpcPerson | None: |
There was a problem hiding this comment.
feels like it should be a reusable utility instead of a one-off private helper (and we might have a way to do this already?)
There was a problem hiding this comment.
yes I will simplify this
|
|
||
| @shared_task | ||
| def create_draft_repos_task(): | ||
| """Create repositories for all queued documents that have none yet. |
There was a problem hiding this comment.
Is this intended as a temporary task or are you planning to keep it around?
There was a problem hiding this comment.
that would be a one-time task for everything in queue, then it could be removed
2 participants
resolve #1326