mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-08 00:03:07 +00:00
fix: total local usage per integration, not per resource type name
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
2a85f4b7d9
commit
078c8d4f1c
@@ -8554,17 +8554,17 @@ paths:
|
||||
- resource_type
|
||||
- count
|
||||
|
||||
/w/{workspace}/resources/type/hub/picked:
|
||||
/w/{workspace}/resources/type/hub/info:
|
||||
get:
|
||||
summary: list the hub's most picked resource types
|
||||
operationId: listHubPickedResourceTypes
|
||||
summary: list what the hub knows about its resource types
|
||||
operationId: listHubResourceTypeInfo
|
||||
tags:
|
||||
- resource
|
||||
parameters:
|
||||
- $ref: "#/components/parameters/WorkspaceId"
|
||||
responses:
|
||||
"200":
|
||||
description: resource types ranked by hub picks, empty if the hub has no such ranking
|
||||
description: each hub resource type with its integration and pick count, empty if the hub answers neither read
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
@@ -8574,10 +8574,14 @@ paths:
|
||||
properties:
|
||||
name:
|
||||
type: string
|
||||
app:
|
||||
description: the integration the resource type belongs to, which is not always its own name
|
||||
type: string
|
||||
picks:
|
||||
type: integer
|
||||
required:
|
||||
- name
|
||||
- app
|
||||
- picks
|
||||
|
||||
/w/{workspace}/resources/type/hub/pick/{name}:
|
||||
|
||||
@@ -90,7 +90,7 @@ pub fn workspaced_service() -> Router {
|
||||
.route("/type/list", get(list_resource_types))
|
||||
.route("/type/listnames", get(list_resource_types_names))
|
||||
.route("/type/resource_counts", get(list_resource_counts_by_type))
|
||||
.route("/type/hub/picked", get(list_hub_picked_resource_types))
|
||||
.route("/type/hub/info", get(list_hub_resource_type_info))
|
||||
.route("/type/hub/pick/{name}", post(pick_hub_resource_type))
|
||||
.route("/type/get/{name}", get(get_resource_type))
|
||||
.route("/type/exists/{name}", get(exists_resource_type))
|
||||
@@ -2618,8 +2618,9 @@ struct HubCached<T> {
|
||||
const HUB_RT_IDS_TTL: std::time::Duration = std::time::Duration::from_secs(60 * 60);
|
||||
const HUB_RT_PICKS_TTL: std::time::Duration = std::time::Duration::from_secs(5 * 60);
|
||||
|
||||
static HUB_RT_IDS: LazyLock<std::sync::RwLock<Option<HubCached<HashMap<String, i64>>>>> =
|
||||
LazyLock::new(|| std::sync::RwLock::new(None));
|
||||
static HUB_RT_IDS: LazyLock<
|
||||
std::sync::RwLock<Option<HubCached<HashMap<String, HubResourceType>>>>,
|
||||
> = LazyLock::new(|| std::sync::RwLock::new(None));
|
||||
static HUB_RT_PICKS: LazyLock<std::sync::RwLock<Option<HubCached<Vec<HubResourceTypePicks>>>>> =
|
||||
LazyLock::new(|| std::sync::RwLock::new(None));
|
||||
|
||||
@@ -2645,17 +2646,30 @@ fn hub_cache_put<T>(cache: &std::sync::RwLock<Option<HubCached<T>>>, hub_base_ur
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct HubResourceTypeId {
|
||||
struct HubResourceTypeEntry {
|
||||
id: i64,
|
||||
name: String,
|
||||
app: String,
|
||||
}
|
||||
|
||||
/// Maps a resource type's name to the id the hub addresses it by. Windmill knows types by
|
||||
/// name — the hub id is not stored anywhere locally — so reporting a pick has to resolve
|
||||
/// one. `None` when the hub cannot be reached or does not answer with a list.
|
||||
async fn hub_resource_type_ids(db: &DB, hub_base_url: &str) -> Option<HashMap<String, i64>> {
|
||||
if let Some(ids) = hub_cache_get(&HUB_RT_IDS, hub_base_url, HUB_RT_IDS_TTL) {
|
||||
return Some(ids);
|
||||
#[derive(Clone)]
|
||||
struct HubResourceType {
|
||||
id: i64,
|
||||
/// The integration the type belongs to. Usually the type's own name, but not always:
|
||||
/// `discord_webhook` and `discord_bot_configuration` are both `discord`, and only the
|
||||
/// hub knows that. Without it a workspace holding a `discord_webhook` resource looks
|
||||
/// like one that has never touched Discord.
|
||||
app: String,
|
||||
}
|
||||
|
||||
/// What the hub knows about every published resource type, keyed by the name Windmill
|
||||
/// addresses it by. `None` when the hub cannot be reached or does not answer with a list.
|
||||
async fn hub_resource_types(
|
||||
db: &DB,
|
||||
hub_base_url: &str,
|
||||
) -> Option<HashMap<String, HubResourceType>> {
|
||||
if let Some(index) = hub_cache_get(&HUB_RT_IDS, hub_base_url, HUB_RT_IDS_TTL) {
|
||||
return Some(index);
|
||||
}
|
||||
let response = windmill_common::utils::http_get_from_hub(
|
||||
&windmill_common::utils::HTTP_CLIENT,
|
||||
@@ -2669,17 +2683,17 @@ async fn hub_resource_type_ids(db: &DB, hub_base_url: &str) -> Option<HashMap<St
|
||||
if !response.status().is_success() {
|
||||
return None;
|
||||
}
|
||||
// Only the ids are kept. That listing carries every type's schema — around a megabyte
|
||||
// — and none of it has anything to do with reporting a pick.
|
||||
let ids: HashMap<String, i64> = response
|
||||
.json::<Vec<HubResourceTypeId>>()
|
||||
// Only the id and the app are kept. That listing carries every type's schema — around
|
||||
// a megabyte — and neither reporting a pick nor grouping types by integration needs it.
|
||||
let index: HashMap<String, HubResourceType> = response
|
||||
.json::<Vec<HubResourceTypeEntry>>()
|
||||
.await
|
||||
.ok()?
|
||||
.into_iter()
|
||||
.map(|rt| (rt.name, rt.id))
|
||||
.map(|rt| (rt.name, HubResourceType { id: rt.id, app: rt.app }))
|
||||
.collect();
|
||||
hub_cache_put(&HUB_RT_IDS, hub_base_url, ids.clone());
|
||||
Some(ids)
|
||||
hub_cache_put(&HUB_RT_IDS, hub_base_url, index.clone());
|
||||
Some(index)
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
@@ -2706,10 +2720,7 @@ async fn pick_hub_resource_type(
|
||||
check_scopes(&authed, || "resources:write".to_string())?;
|
||||
let hub_base_url = (**windmill_common::HUB_BASE_URL.load()).clone();
|
||||
let success = async {
|
||||
let id = hub_resource_type_ids(&db, &hub_base_url)
|
||||
.await?
|
||||
.get(&name)
|
||||
.copied()?;
|
||||
let id = hub_resource_types(&db, &hub_base_url).await?.get(&name)?.id;
|
||||
let response = windmill_common::utils::http_get_from_hub(
|
||||
&windmill_common::utils::HTTP_CLIENT,
|
||||
&format!("{hub_base_url}/resource_types/{id}/pick"),
|
||||
@@ -2766,42 +2777,75 @@ mod hub_picks_tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// The hub's own popularity ranking for resource types. Empty rather than an error when
|
||||
/// the hub has no such endpoint, so the pickers reading this treat an older or private hub
|
||||
/// as "no hub signal" and fall back to what the workspace itself uses.
|
||||
async fn list_hub_picked_resource_types(
|
||||
Extension(db): Extension<DB>,
|
||||
) -> JsonResult<Vec<HubResourceTypePicks>> {
|
||||
let hub_base_url = (**windmill_common::HUB_BASE_URL.load()).clone();
|
||||
if let Some(picks) = hub_cache_get(&HUB_RT_PICKS, &hub_base_url, HUB_RT_PICKS_TTL) {
|
||||
return Ok(Json(picks));
|
||||
}
|
||||
let picks = async {
|
||||
let response = windmill_common::utils::http_get_from_hub(
|
||||
&windmill_common::utils::HTTP_CLIENT,
|
||||
&format!("{hub_base_url}/resource_types/picked"),
|
||||
false,
|
||||
Some(vec![("limit", "200".to_string())]),
|
||||
Some(&db),
|
||||
)
|
||||
.await
|
||||
.ok()?;
|
||||
if !response.status().is_success() {
|
||||
return None;
|
||||
}
|
||||
Some(
|
||||
response
|
||||
.json::<HubPickedResourceTypes>()
|
||||
.await
|
||||
.ok()?
|
||||
.resource_types,
|
||||
)
|
||||
}
|
||||
.await
|
||||
.unwrap_or_default();
|
||||
/// One hub resource type as the pickers need it.
|
||||
#[derive(Serialize, Clone)]
|
||||
struct HubResourceTypeInfo {
|
||||
name: String,
|
||||
/// The integration it belongs to, so a caller can total a workspace's resources per
|
||||
/// integration rather than per type.
|
||||
app: String,
|
||||
picks: i64,
|
||||
}
|
||||
|
||||
hub_cache_put(&HUB_RT_PICKS, &hub_base_url, picks.clone());
|
||||
Ok(Json(picks))
|
||||
/// What the hub knows about its resource types: which integration each belongs to, and how
|
||||
/// often each has been picked.
|
||||
///
|
||||
/// Empty rather than an error when the hub answers neither read, so the pickers treat an
|
||||
/// older or private hub as "no hub signal" and fall back to what the workspace itself uses.
|
||||
/// The two reads degrade independently: a hub that lists types but has no `picked` route
|
||||
/// still supplies the type-to-integration mapping, which is what decides whether a
|
||||
/// workspace's resources are recognised as belonging to an integration at all.
|
||||
async fn list_hub_resource_type_info(
|
||||
Extension(db): Extension<DB>,
|
||||
) -> JsonResult<Vec<HubResourceTypeInfo>> {
|
||||
let hub_base_url = (**windmill_common::HUB_BASE_URL.load()).clone();
|
||||
let picks = match hub_cache_get(&HUB_RT_PICKS, &hub_base_url, HUB_RT_PICKS_TTL) {
|
||||
Some(picks) => picks,
|
||||
None => {
|
||||
let fetched = async {
|
||||
let response = windmill_common::utils::http_get_from_hub(
|
||||
&windmill_common::utils::HTTP_CLIENT,
|
||||
&format!("{hub_base_url}/resource_types/picked"),
|
||||
false,
|
||||
Some(vec![("limit", "200".to_string())]),
|
||||
Some(&db),
|
||||
)
|
||||
.await
|
||||
.ok()?;
|
||||
if !response.status().is_success() {
|
||||
return None;
|
||||
}
|
||||
Some(
|
||||
response
|
||||
.json::<HubPickedResourceTypes>()
|
||||
.await
|
||||
.ok()?
|
||||
.resource_types,
|
||||
)
|
||||
}
|
||||
.await
|
||||
.unwrap_or_default();
|
||||
hub_cache_put(&HUB_RT_PICKS, &hub_base_url, fetched.clone());
|
||||
fetched
|
||||
}
|
||||
};
|
||||
|
||||
let picks_by_name: HashMap<String, i64> =
|
||||
picks.into_iter().map(|rt| (rt.name, rt.picks)).collect();
|
||||
let index = hub_resource_types(&db, &hub_base_url)
|
||||
.await
|
||||
.unwrap_or_default();
|
||||
|
||||
Ok(Json(
|
||||
index
|
||||
.into_iter()
|
||||
.map(|(name, rt)| HubResourceTypeInfo {
|
||||
picks: picks_by_name.get(&name).copied().unwrap_or(0),
|
||||
name,
|
||||
app: rt.app,
|
||||
})
|
||||
.collect(),
|
||||
))
|
||||
}
|
||||
|
||||
async fn get_resource_type(
|
||||
|
||||
@@ -387,9 +387,10 @@
|
||||
}
|
||||
|
||||
/**
|
||||
* Orders the browse list: what the hub sees people pick, then what this workspace already
|
||||
* has resources of. Both signals are fetched, so the rows render in the order the lists
|
||||
* arrived in and re-sort when this lands.
|
||||
* Orders the browse list: the types this workspace already has resources of lead, ranked
|
||||
* among themselves by the hub's pick counts, then everything else on the same counts.
|
||||
* `byPopularity` carries the full rule. Both signals are fetched, so the rows render
|
||||
* alphabetically and re-sort when this lands.
|
||||
*/
|
||||
let popularity: (a: string, b: string) => number = $state(alphabetical)
|
||||
|
||||
@@ -998,6 +999,9 @@
|
||||
if (step == 1) {
|
||||
loadConnects()
|
||||
loadResourceTypes()
|
||||
// Opened on a specific type, `open()` skipped this; backing out to the browse
|
||||
// list is the first time it is needed.
|
||||
loadPopularity()
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@
|
||||
import {
|
||||
alphabetical,
|
||||
byPopularity,
|
||||
localResourceTypeCounts
|
||||
localCountsByIntegration
|
||||
} from '$lib/components/pickerPopularity'
|
||||
|
||||
interface Props {
|
||||
@@ -62,11 +62,12 @@
|
||||
if ($disableHubStore) return
|
||||
try {
|
||||
hubNotAvailable = false
|
||||
const integrations = await IntegrationService.listHubIntegrations({
|
||||
kind: filterKind
|
||||
})
|
||||
// Independent reads, so they share one round trip before first paint.
|
||||
const [integrations, local] = await Promise.all([
|
||||
IntegrationService.listHubIntegrations({ kind: filterKind }),
|
||||
$workspaceStore ? localCountsByIntegration($workspaceStore) : {}
|
||||
])
|
||||
const hubPicks = Object.fromEntries(integrations.map((x) => [x.name, x.picks ?? 0]))
|
||||
const local = $workspaceStore ? await localResourceTypeCounts($workspaceStore) : {}
|
||||
popularity = byPopularity(hubPicks, local)
|
||||
allApps = integrations.map((x) => x.name).sort(popularity)
|
||||
} catch (err) {
|
||||
|
||||
@@ -53,7 +53,7 @@
|
||||
import {
|
||||
alphabetical,
|
||||
byPopularity,
|
||||
localResourceTypeCounts
|
||||
localCountsByIntegration
|
||||
} from '$lib/components/pickerPopularity'
|
||||
|
||||
let customUi: undefined | FlowBuilderWhitelabelCustomUi = getContext('customUi')
|
||||
@@ -112,9 +112,12 @@
|
||||
if ($disableHubStore) return
|
||||
try {
|
||||
hubNotAvailable = false
|
||||
const integrations = await listHubIntegrationsCached({ kind: filterKind, refreshCount })
|
||||
// Independent reads, so they share one round trip before first paint.
|
||||
const [integrations, local] = await Promise.all([
|
||||
listHubIntegrationsCached({ kind: filterKind, refreshCount }),
|
||||
$workspaceStore ? localCountsByIntegration($workspaceStore) : {}
|
||||
])
|
||||
const hubPicks = Object.fromEntries(integrations.map((x) => [x.name, x.picks ?? 0]))
|
||||
const local = $workspaceStore ? await localResourceTypeCounts($workspaceStore) : {}
|
||||
popularity = byPopularity(hubPicks, local)
|
||||
allApps = integrations.map((x) => x.name).sort(popularity)
|
||||
} catch (err) {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { alphabetical, byPopularity } from './pickerPopularity'
|
||||
import { alphabetical, byPopularity, totalLocalCountsByApp } from './pickerPopularity'
|
||||
|
||||
const order = (names: string[], hub: Record<string, number>, local: Record<string, number> = {}) =>
|
||||
[...names].sort(byPopularity(hub, local))
|
||||
@@ -51,3 +51,30 @@ describe('byPopularity', () => {
|
||||
expect(['stripe', 'ably', 'github'].sort(alphabetical)).toEqual(['ably', 'github', 'stripe'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('totalLocalCountsByApp', () => {
|
||||
const HUB = [
|
||||
{ name: 'discord_webhook', app: 'discord', picks: 0 },
|
||||
{ name: 'discord_bot_configuration', app: 'discord', picks: 0 },
|
||||
{ name: 'ms_teams_webhook', app: 'msteams', picks: 0 },
|
||||
{ name: 'slack', app: 'slack', picks: 0 }
|
||||
]
|
||||
|
||||
// The integration pickers list app names, the counts arrive keyed by resource type, and
|
||||
// the two only usually agree. Without the mapping a workspace whose Discord credential is
|
||||
// a `discord_webhook` never reaches the used-here tier at all.
|
||||
it('totals a resource type under the integration it belongs to', () => {
|
||||
expect(totalLocalCountsByApp({ discord_webhook: 2 }, HUB)).toEqual({ discord: 2 })
|
||||
})
|
||||
|
||||
it('sums the several types one integration can have', () => {
|
||||
expect(
|
||||
totalLocalCountsByApp({ discord_webhook: 2, discord_bot_configuration: 1 }, HUB)
|
||||
).toEqual({ discord: 3 })
|
||||
})
|
||||
|
||||
// What a workspace-made type, and an unreachable hub, both leave every entry with.
|
||||
it('keeps a type the hub has no mapping for under its own name', () => {
|
||||
expect(totalLocalCountsByApp({ c_acme: 1, slack: 2 }, HUB)).toEqual({ c_acme: 1, slack: 2 })
|
||||
})
|
||||
})
|
||||
|
||||
@@ -17,13 +17,14 @@ export type PopularityCounts = Record<string, number>
|
||||
*/
|
||||
const CACHE_MS = 60_000
|
||||
|
||||
const hubPicksCached = createCache(
|
||||
async ({ workspace }: { workspace: string }): Promise<PopularityCounts> => {
|
||||
type HubResourceTypeInfo = { name: string; app: string; picks: number }
|
||||
|
||||
const hubInfoCached = createCache(
|
||||
async ({ workspace }: { workspace: string }): Promise<HubResourceTypeInfo[]> => {
|
||||
try {
|
||||
const picked = await ResourceService.listHubPickedResourceTypes({ workspace })
|
||||
return Object.fromEntries(picked.map((rt) => [rt.name, rt.picks]))
|
||||
return await ResourceService.listHubResourceTypeInfo({ workspace })
|
||||
} catch {
|
||||
return {}
|
||||
return []
|
||||
}
|
||||
},
|
||||
{ invalidateMs: CACHE_MS }
|
||||
@@ -46,20 +47,55 @@ const localCountsCached = createCache(
|
||||
* and on an instance that has switched the hub off — a closed environment must not spend a
|
||||
* request on hub.windmill.dev just to order a list.
|
||||
*/
|
||||
export function hubResourceTypePicks(workspace: string): Promise<PopularityCounts> {
|
||||
if (get(disableHubStore)) return Promise.resolve({})
|
||||
return hubPicksCached({ workspace })
|
||||
export async function hubResourceTypePicks(workspace: string): Promise<PopularityCounts> {
|
||||
if (get(disableHubStore)) return {}
|
||||
const info = await hubInfoCached({ workspace })
|
||||
return Object.fromEntries(info.map((rt) => [rt.name, rt.picks]))
|
||||
}
|
||||
|
||||
/**
|
||||
* How many resources of each type this workspace holds — the only evidence about this
|
||||
* particular team, and the same map the flow step picker reads for an integration, since an
|
||||
* integration and the resource type authenticating it share a name.
|
||||
* particular team. Keyed by resource type, which is what the add-resource drawer lists.
|
||||
*/
|
||||
export function localResourceTypeCounts(workspace: string): Promise<PopularityCounts> {
|
||||
return localCountsCached({ workspace })
|
||||
}
|
||||
|
||||
/**
|
||||
* The same counts totalled per integration, which is what the flow step picker lists.
|
||||
*
|
||||
* A type usually shares its integration's name, but often enough it does not:
|
||||
* `discord_webhook` and `discord_bot_configuration` are both Discord, `ms_teams_webhook` and
|
||||
* `azure_bot` are both MS Teams. Only the hub knows that, so a workspace whose Discord
|
||||
* credential is a `discord_webhook` would otherwise read as one that has never touched
|
||||
* Discord — and since local usage is the leading tier, that decides which half of the list
|
||||
* the integration lands in, not merely its position within one.
|
||||
*
|
||||
* A type the hub has no mapping for counts under its own name, which is the right guess and
|
||||
* also what an unreachable hub leaves every type with.
|
||||
*/
|
||||
export async function localCountsByIntegration(workspace: string): Promise<PopularityCounts> {
|
||||
const [counts, info] = await Promise.all([
|
||||
localCountsCached({ workspace }),
|
||||
get(disableHubStore) ? Promise.resolve([]) : hubInfoCached({ workspace })
|
||||
])
|
||||
return totalLocalCountsByApp(counts, info)
|
||||
}
|
||||
|
||||
/** The mapping half of {@link localCountsByIntegration}, separated so it can be tested alone. */
|
||||
export function totalLocalCountsByApp(
|
||||
counts: PopularityCounts,
|
||||
hub: { name: string; app: string }[]
|
||||
): PopularityCounts {
|
||||
const appOf = new Map(hub.map((rt) => [rt.name, rt.app]))
|
||||
const byApp: PopularityCounts = {}
|
||||
for (const [name, count] of Object.entries(counts)) {
|
||||
const app = appOf.get(name) ?? name
|
||||
byApp[app] = (byApp[app] ?? 0) + count
|
||||
}
|
||||
return byApp
|
||||
}
|
||||
|
||||
/**
|
||||
* Tell the hub a resource type was taken into a workspace, which is what its ranking counts.
|
||||
* Fire-and-forget: a hub that does not count picks must not be felt by the user who just
|
||||
|
||||
Reference in New Issue
Block a user