Bug 1154795 - add an endpoint to relengapi that creates and returns a Mozharness archive via S3 #279
Conversation
…ves hgmo and mozharness refs out of generic methods
|
hrm, guess I need to write some docs and tests first :) will squash and update again |
|
hrm, validate (which runs coverage) passes for me locally. I'm not sure if a -0.01% decrease in coverage should be held against me too harshly :) latest pull request contains docs, tests, and pep fixes that was failing in previous PR |
|
True, but we do want to make sure that new code is 100% covered, among other things in https://github.com/mozilla/build-relengapi/blob/master/relengapi/docs/development/contributing.rst The rationale is that such testing is basically the only thing that could prevent a deploy of relengapi from breaking this service in production -- with continuous deployment, we won't have the luxury of manually clicking around on things that we feel our push may have broken. So if there's code not covered by tests, then that code could be silently broken by some unrelated refactor, and we wouldn't know until the trees closed. In this case, it looks like |
|
Having a deeper review look at this now.. |
| For AWS credentials, each bucket should be limited to the AWS IAM role corresponding to the AWS credentials. Buckets in | ||
| the configuration are required to be pre-existing. | ||
|
|
||
| Finally, Archiver avails of Celery. You will need to provide a broker and back-end. |
| def test_accepted_response_when_missing_s3_key(app, client): | ||
| setup_buckets(app, cfg['SUBREPO_MOZHARNESS_CFG']) | ||
| resp = client.get('/archiver/mozharness/9ebd530c5843?repo=mozilla-central®ion=us-east-1') | ||
| eq_(resp.status_code, 202, resp.status) |
There was a problem hiding this comment.
This hits hg too, doesn't it.. it just doesn't fail because the test doesn't poll the task status..
|
Sorry, that got really long. This is in good shape, and most of these comments are relatively minor changes. I think the only controversial idea is to just accept any old path, tack it onto I don't remember if we talked about how these files will be deleted from S3 -- a lifetime configuration on the S3 bucket? If that's the case, are you worried about the race condition where S3 deletes a key in between its existence being checked and the client following the 302 redirect? |
supports subdir query arg and is more generic than mozharness. copying raw gzip response not working fixes gzip response, rewrites tests, docs, and addresses pep errors force response decode, update doc usage to reflect changes
| } | ||
|
|
||
| ARCHIVER_S3_BUCKETS = { | ||
| 'us-east-1', 'archiver-us-east-1', |
|
Two one-character changes in the docs and this is good to go :) |
Bug 1154795 - add an endpoint to relengapi that creates and returns a Mozharness archive via S3
this consolidates my relengapi repo into the now unified build-relengapi and addresses feedback comments regarding:
from feedback that is not implemented: