Mrc 5585 submit endpoint #6
Conversation
| @@ -28,5 +28,15 @@ | |||
| porcelain::porcelain_state(root = state$root), | |||
| returning = porcelain::porcelain_returning_json("report_parameters"), | |||
| validate = validate) | |||
| }, | |||
| "POST /report/run" = function(state, validate) { | |||
There was a problem hiding this comment.
Perhaps copy in this file: https://github.com/jameel-institute/daedalus.api/blob/main/.gitattributes
and you'll get nicer rendering of diffs
inst/schema/report_run_request.json
Outdated
| "properties": {}, | ||
| "additionalProperties": true |
There was a problem hiding this comment.
| "properties": {}, | |
| "additionalProperties": true | |
| "additionalProperties": { | |
| "type": ["boolean", "integer", "number", "string"] | |
| } |
we can be stricter here, and the properties key does nothing in the current version
inst/schema/report_run_response.json
Outdated
| "type": "string" | ||
| } | ||
| }, | ||
| "required": ["job_id"], |
There was a problem hiding this comment.
any reason not to return as a string rather than an object? if an object any reason not to use jobId as that's probably closer to what Packit uses? And any reason we can't use task not job as that's what rrq uses?
There was a problem hiding this comment.
no reason other than i was speculating that we may want to add other things that return from this endpoint, and having an object returned means an easier future refactor in kotlin if another field is needed
yep this can definitely be camelcase
job_id was only used because I believe the sequence diagram uses job id and that percolated through to the tickets, i can change it to task!
There was a problem hiding this comment.
have changed to taskId
| @@ -1,3 +1,3 @@ | |||
| orderly2::orderly_artefact("Some data", "data.rds") | |||
| orderly2::orderly_artefact(description = "Some data", "data.rds") | |||
There was a problem hiding this comment.
sorry, this is for the greater good...
| "branch": { | ||
| "type": "string" | ||
| }, | ||
| "hash": { | ||
| "type": "string" | ||
| }, |
There was a problem hiding this comment.
why do we have both branch and hash, and not ref? Probably this is a good idea though tbh
What is the interpretation of branch names? Do we add main/ to them?
There was a problem hiding this comment.
the runner checks out to that branch and resets head to the hash, we would have a different behaviour if we used a ref right? would we not get a detached head if someone specified just a hash
there will be some extra work in the UI if we want people to be able to specify a commit hash on a branch instead of just a commit hash but that can come later
branch names are just what we do a git checkout to before resetting to the specified hash, im not sure what you mean by adding main/ to them but we dont do anything like that, just pass branchname into git checkout via gert
The implementation of the submit function was done in #5, this is wrapping that up in an endpoint and exposing it. Main changes include:
submit_report_runendpoint