Skip to content

Commit

Permalink
Revert "Update BundleUpload status for observability (#99)"
Browse files Browse the repository at this point in the history
This reverts commit f81ac92.
  • Loading branch information
gnalh authored Oct 3, 2024
1 parent f81ac92 commit 088deb4
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 118 deletions.
33 changes: 2 additions & 31 deletions cli/src/clients.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use std::path::PathBuf;
use anyhow::Context;

use crate::types::{
BundleUploadStatus, CreateBundleUploadRequest, CreateBundleUploadResponse, CreateRepoRequest,
GetQuarantineBulkTestStatusRequest, QuarantineConfig, Repo, UpdateBundleUploadRequest,
CreateBundleUploadRequest, CreateBundleUploadResponse, CreateRepoRequest,
GetQuarantineBulkTestStatusRequest, QuarantineConfig, Repo,
};
use crate::utils::status_code_help;

Expand Down Expand Up @@ -47,35 +47,6 @@ pub async fn create_trunk_repo(

Ok(())
}
pub async fn update_bundle_upload_status(
origin: &str,
api_token: &str,
id: &str,
upload_status: &BundleUploadStatus,
) -> anyhow::Result<()> {
let client = reqwest::Client::new();
let resp = client
.patch(format!("{}/v1/metrics/updateBundleUpload", origin))
.timeout(TRUNK_API_TIMEOUT)
.header(reqwest::header::CONTENT_TYPE, "application/json")
.header(TRUNK_API_TOKEN_HEADER, api_token)
.json(&UpdateBundleUploadRequest {
id: id.to_owned(),
upload_status: upload_status.to_owned(),
})
.send()
.await
.map_err(|e| anyhow::anyhow!(e).context("Failed to update bundle upload status"))?;

if resp.status() != reqwest::StatusCode::OK {
return Err(
anyhow::anyhow!("{}: {}", resp.status(), status_code_help(resp.status()))
.context("Failed to update bundle upload status"),
);
}

Ok(())
}

pub async fn create_bundle_upload_intent(
origin: &str,
Expand Down
39 changes: 5 additions & 34 deletions cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use tokio_retry::strategy::ExponentialBackoff;
use tokio_retry::Retry;
use trunk_analytics_cli::bundler::BundlerUtil;
use trunk_analytics_cli::clients::{
create_bundle_upload_intent, create_trunk_repo, put_bundle_to_s3, update_bundle_upload_status,
create_bundle_upload_intent, create_trunk_repo, put_bundle_to_s3,
};
use trunk_analytics_cli::codeowners::CodeOwners;
use trunk_analytics_cli::constants::{
Expand All @@ -18,8 +18,7 @@ use trunk_analytics_cli::runner::{
};
use trunk_analytics_cli::scanner::{BundleRepo, EnvScanner};
use trunk_analytics_cli::types::{
BundleMeta, BundleUploadStatus, QuarantineBulkTestStatus, QuarantineRunResult, RunResult,
META_VERSION,
BundleMeta, QuarantineBulkTestStatus, QuarantineRunResult, RunResult, META_VERSION,
};
use trunk_analytics_cli::utils::{from_non_empty_or_default, parse_custom_tags};

Expand Down Expand Up @@ -257,7 +256,7 @@ async fn run_upload(
),
org: org_url_slug.clone(),
repo: repo.clone(),
bundle_upload_id: upload.id.clone(),
bundle_upload_id: upload.id,
tags,
file_sets,
envs,
Expand Down Expand Up @@ -296,42 +295,14 @@ async fn run_upload(
log::info!("Flushed temporary tarball to {:?}", bundle_time_file);

if dry_run {
if let Err(e) = update_bundle_upload_status(
&api_address,
&token,
&upload.id,
&BundleUploadStatus::DryRun,
)
.await
{
log::warn!("Failed to update bundle upload status: {}", e);
} else {
log::debug!("Updated bundle upload status to DRY_RUN");
}
log::info!("Dry run, skipping upload.");
return Ok(exit_code);
}

let upload_status = Retry::spawn(default_delay(), || {
Retry::spawn(default_delay(), || {
put_bundle_to_s3(&upload.url, &bundle_time_file)
})
.await
.map(|_| BundleUploadStatus::UploadComplete)
.unwrap_or_else(|e| {
log::error!("Failed to upload bundle to S3 after retries: {}", e);
BundleUploadStatus::UploadFailed
});
if let Err(e) =
update_bundle_upload_status(&api_address, &token, &upload.id, &upload_status).await
{
log::warn!(
"Failed to update bundle upload status to {:#?}: {}",
upload_status,
e
)
} else {
log::debug!("Updated bundle upload status to {:#?}", upload_status)
}
.await?;

let remote_urls = vec![repo.repo_url.clone()];
Retry::spawn(default_delay(), || {
Expand Down
19 changes: 0 additions & 19 deletions cli/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,25 +33,6 @@ pub struct CreateBundleUploadRequest {
pub org_url_slug: String,
}

#[derive(Debug, Serialize, Clone, Deserialize, PartialEq, Eq)]
pub enum BundleUploadStatus {
#[serde(rename = "PENDING")]
Pending,
#[serde(rename = "UPLOAD_COMPLETE")]
UploadComplete,
#[serde(rename = "UPLOAD_FAILED")]
UploadFailed,
#[serde(rename = "DRY_RUN")]
DryRun,
}

#[derive(Debug, Serialize, Clone, Deserialize, PartialEq, Eq)]
pub struct UpdateBundleUploadRequest {
pub id: String,
#[serde(rename = "uploadStatus")]
pub upload_status: BundleUploadStatus,
}

#[derive(Debug, Serialize, Clone, Deserialize, PartialEq, Eq)]
pub struct GetQuarantineBulkTestStatusRequest {
pub repo: Repo,
Expand Down
25 changes: 2 additions & 23 deletions cli/tests/test_utils/mock_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,20 @@ use axum::body::Bytes;
use axum::extract::State;
use axum::http::StatusCode;
use axum::response::Response;
use axum::routing::{any, patch, post, put};
use axum::routing::{any, post, put};
use axum::{Json, Router};
use tempfile::tempdir;
use tokio::net::TcpListener;
use tokio::spawn;
use trunk_analytics_cli::types::{
CreateBundleUploadRequest, CreateBundleUploadResponse, CreateRepoRequest,
GetQuarantineBulkTestStatusRequest, QuarantineConfig, UpdateBundleUploadRequest,
GetQuarantineBulkTestStatusRequest, QuarantineConfig,
};

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum RequestPayload {
CreateRepo(CreateRepoRequest),
CreateBundleUpload(CreateBundleUploadRequest),
UpdateBundleUpload(UpdateBundleUploadRequest),
GetQuarantineBulkTestStatus(GetQuarantineBulkTestStatusRequest),
S3Upload(PathBuf),
}
Expand Down Expand Up @@ -55,10 +54,6 @@ pub async fn spawn_mock_server() -> SharedMockServerState {
"/v1/metrics/createBundleUpload",
post(create_bundle_handler),
)
.route(
"/v1/metrics/updateBundleUpload",
patch(update_bundle_handler),
)
.route(
"/v1/metrics/getQuarantineConfig",
post(get_quarantining_config_handler),
Expand Down Expand Up @@ -121,22 +116,6 @@ async fn create_bundle_handler(
})
}

#[allow(dead_code)] // TODO: move this to its own crate to get rid of the need for this
#[axum::debug_handler]
async fn update_bundle_handler(
State(state): State<SharedMockServerState>,
Json(update_bundle_upload_request): Json<UpdateBundleUploadRequest>,
) -> Response<String> {
state
.requests
.lock()
.unwrap()
.push(RequestPayload::UpdateBundleUpload(
update_bundle_upload_request,
));
Response::new(String::from("OK"))
}

#[allow(dead_code)] // TODO: move this to its own crate to get rid of the need for this
#[axum::debug_handler]
async fn get_quarantining_config_handler(
Expand Down
14 changes: 3 additions & 11 deletions cli/tests/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use test_utils::mock_git_repo::setup_repo_with_commit;
use test_utils::mock_server::{spawn_mock_server, RequestPayload};
use trunk_analytics_cli::codeowners::CodeOwners;
use trunk_analytics_cli::types::{
BundleMeta, BundleUploadStatus, CreateBundleUploadRequest, CreateRepoRequest, FileSetType,
GetQuarantineBulkTestStatusRequest, Repo, UpdateBundleUploadRequest,
BundleMeta, CreateBundleUploadRequest, CreateRepoRequest, FileSetType,
GetQuarantineBulkTestStatusRequest, Repo,
};

mod test_utils;
Expand Down Expand Up @@ -65,7 +65,7 @@ async fn upload_bundle() {
.failure();

let requests = state.requests.lock().unwrap().clone();
assert_eq!(requests.len(), 5);
assert_eq!(requests.len(), 4);
let mut requests_iter = requests.into_iter();

assert_eq!(
Expand Down Expand Up @@ -156,14 +156,6 @@ async fn upload_bundle() {
assert_eq!(bundled_file.owners, ["@user"]);
assert_eq!(bundled_file.team, None);

assert_eq!(
requests_iter.next().unwrap(),
RequestPayload::UpdateBundleUpload(UpdateBundleUploadRequest {
id: "test-bundle-upload-id".to_string(),
upload_status: BundleUploadStatus::UploadComplete
}),
);

assert_eq!(
requests_iter.next().unwrap(),
RequestPayload::CreateRepo(CreateRepoRequest {
Expand Down

0 comments on commit 088deb4

Please sign in to comment.