Skip to content

Commit

Permalink
merge: #3099 #3100
Browse files Browse the repository at this point in the history
3099: Cache component name in fix to optimize fetching them r=paulocsanz a=paulocsanz



3100: Enable compression in services r=paulocsanz a=paulocsanz



Co-authored-by: Paulo Cabral <[email protected]>
  • Loading branch information
si-bors-ng[bot] and paulocsanz authored Dec 22, 2023
3 parents 089597d + 906eaa4 + 5f7e24b commit 98bf661
Show file tree
Hide file tree
Showing 17 changed files with 325 additions and 87 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ tokio-util = { version = "0.7.8", features = ["codec"] }
tokio-vsock = { version = "0.4.0"}
toml = { version = "0.7.6" }
tower = "0.4.13"
tower-http = { version = "0.4.0", features = ["cors", "trace"] }
tower-http = { version = "0.4", features = ["cors", "trace", "compression-br", "compression-deflate", "compression-gzip"] }
tracing = { version = "0.1" }
tracing-opentelemetry = "0.18.0"
tracing-subscriber = { version = "0.3.17", features = ["env-filter", "std"] }
Expand Down
41 changes: 14 additions & 27 deletions app/web/src/components/NoSelectionDetailsPanel.vue
Original file line number Diff line number Diff line change
Expand Up @@ -30,32 +30,20 @@
</div>
</template>

<template v-if="componentsStore.allComponents.length === 0">
<div class="flex flex-col items-center text-neutral-400 pt-lg">
<EmptyStateIcon name="no-assets" class="mt-3" />
<span class="text-xl dark:text-neutral-300">Your Model Is Empty</span>
<div class="capsize px-xs py-md italic text-sm text-center">
Drag some assets onto the diagram
</div>
</div>
</template>

<template v-else>
<div class="absolute inset-0">
<TabGroup startSelectedTabSlug="changes">
<TabGroupItem label="Changes" slug="changes">
<ChangesPanel />
</TabGroupItem>
<TabGroupItem
v-if="featureFlagsStore.SECRETS_MANAGEMENT"
label="Secrets"
slug="secrets"
>
<SecretsPanel />
</TabGroupItem>
</TabGroup>
</div>
</template>
<div class="absolute inset-0">
<TabGroup startSelectedTabSlug="changes">
<TabGroupItem label="Changes" slug="changes">
<ChangesPanel />
</TabGroupItem>
<TabGroupItem
v-if="featureFlagsStore.SECRETS_MANAGEMENT"
label="Secrets"
slug="secrets"
>
<SecretsPanel />
</TabGroupItem>
</TabGroup>
</div>
</ScrollArea>
</template>

Expand All @@ -74,7 +62,6 @@ import { useComponentsStore } from "@/store/components.store";
import { useChangeSetsStore } from "@/store/change_sets.store";
import { useFeatureFlagsStore } from "@/store/feature_flags.store";
import { useActionsStore } from "@/store/actions.store";
import EmptyStateIcon from "./EmptyStateIcon.vue";
import SidebarSubpanelTitle from "./SidebarSubpanelTitle.vue";
import ChangesPanel from "./ChangesPanel.vue";
import SecretsPanel from "./SecretsPanel.vue";
Expand Down
1 change: 0 additions & 1 deletion app/web/src/store/fixes.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export type Fix = {
componentName: string;
componentId: ComponentId;
attributeValueId: AttributeValueId;
provider?: string;
resource?: Resource | null;
startedAt?: string;
finishedAt?: string;
Expand Down
4 changes: 3 additions & 1 deletion lib/cyclone-server/src/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::sync::Arc;
use axum::{routing::get, Extension, Router};
use telemetry::prelude::*;
use tokio::sync::mpsc;
use tower_http::compression::CompressionLayer;

use crate::{
extract::RequestLimiter,
Expand All @@ -25,7 +26,8 @@ pub fn routes(
"/readiness",
get(handlers::readiness).head(handlers::readiness),
)
.nest("/execute", execute_routes(config, shutdown_tx.clone()));
.nest("/execute", execute_routes(config, shutdown_tx.clone()))
.layer(CompressionLayer::new());

if let Some(watch_timeout) = config.watch() {
debug!("enabling watch endpoint");
Expand Down
27 changes: 19 additions & 8 deletions lib/dal/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use chrono::{DateTime, Utc};
use serde::{Deserialize, Serialize};
use serde_json::Value;
use si_data_nats::NatsError;
use si_data_pg::PgError;
use std::collections::HashMap;
Expand Down Expand Up @@ -590,15 +589,27 @@ impl Component {
/// Return the name of the [`Component`](Self) for the provided [`ComponentId`](Self).
#[instrument(skip_all)]
pub async fn find_name(ctx: &DalContext, component_id: ComponentId) -> ComponentResult<String> {
let row = ctx
.txns()
let component_name = ComponentView::new(ctx, component_id)
.await?
.pg()
.query_one(FIND_NAME, &[ctx.tenancy(), ctx.visibility(), &component_id])
.await?;
let component_name: Value = row.try_get("component_name")?;
.properties
.pointer("/si/name")
.cloned()
.unwrap_or(serde_json::Value::Null);

let component_name: Option<String> = serde_json::from_value(component_name)?;
let component_name = component_name.ok_or(ComponentError::NameIsUnset(component_id))?;
let component_name = if let Some(name) = component_name {
name
} else {
let row = ctx
.txns()
.await?
.pg()
.query_one(FIND_NAME, &[ctx.tenancy(), ctx.visibility(), &component_id])
.await?;
let component_name: serde_json::Value = row.try_get("component_name")?;
let component_name: Option<String> = serde_json::from_value(component_name)?;
component_name.ok_or(ComponentError::NameIsUnset(component_id))?
};
Ok(component_name)
}

Expand Down
55 changes: 17 additions & 38 deletions lib/dal/src/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use thiserror::Error;

use crate::fix::batch::FixBatchId;
use crate::func::binding_return_value::FuncBindingReturnValueError;
use crate::schema::SchemaUiMenu;
use crate::{
func::backend::js_action::ActionRunResult, impl_standard_model, pk, standard_model,
standard_model_accessor, standard_model_accessor_ro, standard_model_belongs_to, ActionId,
Expand Down Expand Up @@ -136,6 +135,7 @@ pub struct Fix {

/// The [`Component`](crate::Component) being fixed.
component_id: ComponentId,
component_name: String,
/// The [`ActionPrototype`](crate::action_prototype::ActionPrototype) that runs the action for
/// this fix.
action_prototype_id: ActionPrototypeId,
Expand Down Expand Up @@ -177,18 +177,20 @@ impl Fix {
ctx: &DalContext,
fix_batch_id: FixBatchId,
component_id: ComponentId,
component_name: String,
action_prototype_id: ActionPrototypeId,
) -> FixResult<Self> {
let row = ctx
.txns()
.await?
.pg()
.query_one(
"SELECT object FROM fix_create_v1($1, $2, $3, $4)",
"SELECT object FROM fix_create_v2($1, $2, $3, $4, $5)",
&[
ctx.tenancy(),
ctx.visibility(),
&component_id,
&component_name,
&action_prototype_id,
],
)
Expand All @@ -199,6 +201,7 @@ impl Fix {
}

standard_model_accessor_ro!(component_id, ComponentId);
standard_model_accessor_ro!(component_name, String);
standard_model_accessor_ro!(action_prototype_id, ActionPrototypeId);
standard_model_accessor_ro!(action_kind, ActionKind);
standard_model_accessor!(started_at, Option<String>, FixResult);
Expand Down Expand Up @@ -356,11 +359,20 @@ impl Fix {
}
};
// Gather component-related information, even if the component has been deleted.
let (component_name, schema_name, category) =
Self::component_details_for_history_view(ctx, self.component_id).await?;
let component =
Component::get_by_id(&ctx.clone_with_delete_visibility(), self.component_id())
.await?
.ok_or_else(|| ComponentError::NotFound(*self.component_id()))?;
let schema_name = component
.schema(&ctx.clone_with_delete_visibility())
.await?
.ok_or_else(|| ComponentError::NoSchema(*self.component_id()))?
.name()
.to_owned();

let mut display_name = self.action_kind().clone().to_string();
let action_prototype = ActionPrototype::get_by_id(ctx, self.action_prototype_id()).await?;

if let Some(ap) = action_prototype {
let func_details = Func::get_by_id(ctx, &ap.func_id()).await?;
if let Some(func) = func_details {
Expand All @@ -382,45 +394,13 @@ impl Fix {
action_kind: *self.action_kind(),
display_name,
schema_name,
component_name,
component_name: self.component_name().to_owned(),
component_id: self.component_id,
provider: category,
resource: resource.map(ResourceView::new),
started_at: self.started_at().map(|s| s.to_string()),
finished_at: self.finished_at().map(|s| s.to_string()),
}))
}

/// Gather details related to the [`Component`](crate::Component) for assembling a
/// [`FixHistoryView`].
///
/// This private method should only be called by [`Self::history_view`].
async fn component_details_for_history_view(
ctx: &DalContext,
component_id: ComponentId,
) -> FixResult<(String, String, Option<String>)> {
// For the component-related information, we want to ensure that we can gather the
// fix history view if the component has been deleted. This is helpful if deletion fixes
// fail and the component still needs to be deleted.
let ctx_with_deleted = &ctx.clone_with_delete_visibility();

let component = Component::get_by_id(ctx_with_deleted, &component_id)
.await?
.ok_or_else(|| ComponentError::NotFound(component_id))?;
let schema = component
.schema(ctx_with_deleted)
.await?
.ok_or_else(|| ComponentError::NoSchema(component_id))?;
let category = SchemaUiMenu::find_for_schema(ctx_with_deleted, *schema.id())
.await?
.map(|um| um.category().to_string());

Ok((
component.name(ctx_with_deleted).await?,
schema.name().to_owned(),
category,
))
}
}

#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)]
Expand All @@ -433,7 +413,6 @@ pub struct FixHistoryView {
schema_name: String,
component_name: String,
component_id: ComponentId,
provider: Option<String>,
started_at: Option<String>,
finished_at: Option<String>,
resource: Option<ResourceView>,
Expand Down
35 changes: 35 additions & 0 deletions lib/dal/src/migrations/U2406__fix_component_name.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
ALTER TABLE fixes ADD COLUMN component_name TEXT;
UPDATE fixes SET component_name = '';
ALTER TABLE fixes ALTER COLUMN component_name SET NOT NULL;

CREATE OR REPLACE FUNCTION fix_create_v2(
this_tenancy jsonb,
this_visibility jsonb,
this_component_id ident,
this_component_name text,
this_action_prototype_id ident,
OUT object json) AS
$$
DECLARE
this_tenancy_record tenancy_record_v1;
this_visibility_record visibility_record_v1;
this_new_row fixes%ROWTYPE;
this_action_kind text;
BEGIN
this_tenancy_record := tenancy_json_to_columns_v1(this_tenancy);
this_visibility_record := visibility_json_to_columns_v1(this_visibility);

SELECT
INTO STRICT this_action_kind
kind FROM action_prototypes_v1($1, $2) where id = this_action_prototype_id;

INSERT INTO fixes (tenancy_workspace_pk, visibility_change_set_pk,
component_id, component_name, action_prototype_id, action_kind)
VALUES (this_tenancy_record.tenancy_workspace_pk,
this_visibility_record.visibility_change_set_pk,
this_component_id, this_component_name, this_action_prototype_id, this_action_kind)
RETURNING * INTO this_new_row;

object := row_to_json(this_new_row);
END
$$ LANGUAGE PLPGSQL VOLATILE;
4 changes: 3 additions & 1 deletion lib/module-index-server/src/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use hyper::StatusCode;
use serde_json::{json, Value};
use si_data_pg::PgError;
use thiserror::Error;
use tower_http::compression::CompressionLayer;
use tower_http::cors::CorsLayer;

mod download_builtin_route;
Expand Down Expand Up @@ -54,7 +55,8 @@ pub fn routes(state: AppState) -> Router {
post(reject_module_route::reject_module),
)
.layer(CorsLayer::permissive())
.layer(DefaultBodyLimit::max(MAX_UPLOAD_BYTES));
.layer(DefaultBodyLimit::max(MAX_UPLOAD_BYTES))
.layer(CompressionLayer::new());

router.with_state(state)
}
Expand Down
4 changes: 3 additions & 1 deletion lib/sdf-server/src/server/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use serde_json::{json, Value};
use si_data_nats::NatsError;
use si_data_pg::PgError;
use thiserror::Error;
use tower_http::compression::CompressionLayer;
use tower_http::cors::CorsLayer;

use super::{server::ServerError, state::AppState};
Expand Down Expand Up @@ -47,7 +48,8 @@ pub fn routes(state: AppState) -> Router {
"/api/variant_def",
crate::server::service::variant_definition::routes(),
)
.nest("/api/ws", crate::server::service::ws::routes());
.nest("/api/ws", crate::server::service::ws::routes())
.layer(CompressionLayer::new());

// Load dev routes if we are in dev mode (decided by "opt-level" at the moment).
router = dev_routes(router);
Expand Down
11 changes: 9 additions & 2 deletions lib/sdf-server/src/server/service/change_set/apply_change_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use axum::extract::OriginalUri;
use axum::Json;
use dal::job::definition::{FixItem, FixesJob};
use dal::{
action::ActionBag, ActionId, ChangeSet, ChangeSetPk, Fix, FixBatch, FixId, HistoryActor,
StandardModel, User,
action::ActionBag, ActionId, ChangeSet, ChangeSetPk, Component, ComponentError, Fix, FixBatch,
FixId, HistoryActor, StandardModel, User,
};
use serde::{Deserialize, Serialize};
use std::collections::{HashMap, VecDeque};
Expand Down Expand Up @@ -85,10 +85,17 @@ pub async fn apply_change_set(
}
}

let component = Component::get_by_id(
&ctx.clone_with_delete_visibility(),
bag.action.component_id(),
)
.await?
.ok_or_else(|| ComponentError::NotFound(*bag.action.component_id()))?;
let fix = Fix::new(
&ctx,
*batch.id(),
*bag.action.component_id(),
component.name(&ctx).await?,
*bag.action.action_prototype_id(),
)
.await?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ pub async fn paste_components(
let mut pasted_components_by_original = HashMap::new();
for component_id in &request.component_ids {
let ctx_builder = ctx.to_builder();
let (visibility, component_id) = (request.visibility, *component_id);
let (visibility, component_id) = (*ctx.visibility(), *component_id);
let (offset_x, offset_y) = (request.offset_x, request.offset_y);
let (original_uri, posthog_client) =
(original_uri.clone(), PosthogClient(posthog_client.clone()));
Expand Down
Loading

0 comments on commit 98bf661

Please sign in to comment.