← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] ~bowenfan/turnip:SN2052-add_ref_creation_endpoint_to_turnip_api into turnip:master

 

Hi William, I've rewritten this as a bulk endpoint and the client is now required to pass in the full ref path for each ref creation request, e.g. "refs/tags/1234". The MP is ready for another look now.

Some design choices/considerations:

- Entire request will be rejected on the first schema validation failure. Nothing will be processed.
  
  Request can be partially fulfilled. This will continue processing requests even after hitting a
  repo-error e.g. invalid commit.

  This behaviour is consistent with services that have schema-based validators.

  Rejected alternative 1: Try to continue on API validation failure - This can make things messy 
  because we might not be able to label the errors, e.g. when the "ref" itself is missing.

  Rejected alternative 2: LBYL/dry run the logic layer and refuse the entire batch if there's an error.
  This makes things slower and still does not guarantee strict transactionality. Also we'll need to
  write our own dry run logic, e.g. regex validator for valid ref names.

- Endpoint request body: 
  JSON array of request dicts, e.g. [{"ref":"refs/tags/1234", "commit_sha1":<hash>}, {...}, ...]

- Endpoint response body:
  JSON dict of dicts, e.g. {"created":{"refs/tags/1234":<hash>}, "errors":{"refs/tags/5678":<err_msg>}}
  
  I think we need to at least return the errors because the client needs to know what to retry. For 
  completeness I'm also echoing what's successfully created.

I'll squash the commits and rewrite the merge commit message when this is finalized.

Thanks for your time!
-- 
https://code.launchpad.net/~bowenfan/turnip/+git/turnip/+merge/452540
Your team Launchpad code reviewers is requested to review the proposed merge of ~bowenfan/turnip:SN2052-add_ref_creation_endpoint_to_turnip_api into turnip:master.



References