kaxil commented on code in PR #76747:
URL: https://github.com/apache/airflow/pull/76747#discussion_r4514299436


##########
providers/common/ai/src/airflow/providers/common/ai/plugins/www/src/model-api.ts:
##########
@@ -79,20 +88,19 @@ export function createModelApi(
   region?: TaskRegion,
 ) {
   return {
-    fetchModelName: () =>
-      fetchXComValue<string>(dagId, runId, taskId, mapIndex, 
MODEL_NAME_XCOM_KEY, region),
-    fetchUsage: async () => {
+    fetchModelName: async (): Promise<string | OffloadedValue | null> => {
+      const value = await fetchXComValue<string>(dagId, runId, taskId, 
mapIndex, MODEL_NAME_XCOM_KEY, region);
+      // A reference is a string too, but a model name never holds a URI 
scheme.

Review Comment:
   A model name can hold a URI scheme. This XCom is whatever the provider 
returns in `result.response.model_name`, and vLLM serves the `--model` value as 
the name unless `--served-model-name` is set, so `vllm serve 
s3://bucket/llama-3-8b --load-format runai_streamer` should come back as 
`s3://bucket/llama-3-8b`. With the default DB backend the tab would then say 
the model name is in external storage. Rare, but is there a tighter check, or 
should the comment drop the "never"?



##########
providers/common/ai/src/airflow/providers/common/ai/plugins/www/tests/model.test.mjs:
##########
@@ -195,3 +195,21 @@ test("a skill badge is a button, so its details are 
reachable from the keyboard"
   const badge = document.querySelector('button[aria-label="About the 
sql-review skill"]');
   assert.equal(badge?.textContent, "sql-review");
 });
+
+test("values kept in external storage are listed by their reference instead of 
rendered", async (t) => {

Review Comment:
   This only covers everything offloaded, which needs a threshold of `0`. With 
a positive threshold common.io offloads each value by size, so the usual case 
is skills offloaded while the model name and usage stay inline, and nothing 
asserts the badge and stat boxes still render then. Could this serve only 
skills as a reference and check the other two render?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to