[TRUNK-12896] Include bundle_upload_id in meta.json on test bundle upload #105
Conversation
cli/src/main.rs
Outdated
| }) | ||
| .await?; | ||
|
|
||
| let upload = upload_op.context("Failed to create bundle upload.")?; |
There was a problem hiding this comment.
Was looking for a way to set upload here to a CreateBundleUploadResponse from an Option<CreateBundleUploadResponse>, failing otherwise, since I don't think we'd want to continue if upload_op were None. LMK if that's an incorrect assumption. Based on this SO thread :) https://stackoverflow.com/questions/69738600/simplest-way-to-unwrap-an-option-and-return-error-if-none-anyhow
There was a problem hiding this comment.
This is good, but I think we can do one better :)
In create_bundle_upload_intent I think we can and should change Option<CreateBundleUploadResponse> to just CreateBundleUploadResponse and then you won't need to do that here. That will also require changing from checking for is_success to checking if the API call is not successful (look at get_quarantining_config for example). This also makes create_bundle_upload_intent more consistent with our other API calls like get_quarantining_config. Eventually, I'll get around to cleaning this up so there's a better, more consistent pattern to all of the API calls with retries we make.
Removing the Option<T> type is a little auto-magical because it's how serde implicitly figures out how to deserialize the response using auto-implemented traits:
https://docs.rs/serde/latest/serde/de/index.html#implementations-of-deserialize-provided-by-serde
There was a problem hiding this comment.
Updated! lmk how it looks
max-trunk
requested review from
dfrankland,
TylerJang27 and
tenesttang-trunk
|
✅ 75 passed ⋅ (learn more) |
…adResponse> -> CreateBundleUploadResponse
Includes
bundle_upload_idin themeta.jsonmetadata file to help facilitate making that value accessible by the ETL job. Also renames a couple function / type names