[AIGTWY-4564] Add Claude parent schema discovery - #540
Conversation
…laude # Conflicts: # src/ucode/cli.py
7e57c4c to
914eef7
Compare
andy-xu-db
changed the title
914eef7 to
7e57c4c
Compare
andy-xu-db
changed the title
| ] | ||
| if provider: | ||
| header_lines.append(f"Databricks-Model-Provider-Service: {provider}") | ||
| elif parent_schema: |
There was a problem hiding this comment.
add a comment saying why you can't have both a provider service head and a model service parent schema
| if parent_schema is not None: | ||
| parts = parent_schema.split(".") | ||
| if len(parts) != 2 or any( | ||
| not part or any(ch in " /" or ord(ch) < 32 or ord(ch) == 127 for ch in part) | ||
| for part in parts | ||
| ): | ||
| raise RuntimeError("--parent must be `<catalog>.<schema>`.") |
There was a problem hiding this comment.
this looks....concerning. can you see if ucode already has some validation like this? if no, can you put it into a string utils file and add unit tests for that util file only
There was a problem hiding this comment.
moved to string utils file
|
can u test with a bedrock provider that has claude and gpt models as well? |
| "x-databricks-use-coding-agent-mode", | ||
| "user-agent", | ||
| "databricks-model-provider-service", | ||
| "databricks-model-service-parent-schema", |
There was a problem hiding this comment.
looks like theres a const in the codex PR? can we share that across both places and put into a const file? https://github.com/databricks/unity-gateway/pull/541/changes/f914af76d0f6319127b2cbdbd5564622abbeb40e..f456c19c08ae51cf1493d8ac763246897258aff5#diff-6dfc964820cd6e8cfb07173a500b45493cf33dbb9d62bd6c6a1439d5d89734b8R62
There was a problem hiding this comment.
added as constants in const file
| ) | ||
| if managed_provider: | ||
| provider = managed_provider | ||
| effective_parent_schema = None if provider else parent_schema |
There was a problem hiding this comment.
should we raise an error if someone tries to pass a parent_schema and a provider at the same time?
| str | None, | ||
| typer.Option( | ||
| "--parent", | ||
| help="Discover model services in `<catalog>.<schema>`.", |
There was a problem hiding this comment.
can you put an ex in the help text? Discover model services in .. Example: main.default
| MODEL_PROVIDER_SERVICE_HEADER = "databricks-model-provider-service" | ||
| MODEL_SERVICE_PARENT_SCHEMA_HEADER = "databricks-model-service-parent-schema" |
| if provider: | ||
| return {MODEL_PROVIDER_SERVICE_HEADER: provider} | ||
| if parent_schema: | ||
| return {MODEL_SERVICE_PARENT_SCHEMA_HEADER: parent_schema} | ||
| return {} |
There was a problem hiding this comment.
can you throw an error or smth if both are set? bc right now if u set provider and not parent_schema, it will pick provider and parent_schema will be silently ignored
also is a separate file + util necessary for this? seems like it can be inlined
| ) | ||
| headers = overlay["env"]["ANTHROPIC_CUSTOM_HEADERS"] | ||
| assert "Databricks-Model-Provider-Service: c.s.mps" in headers | ||
| assert "databricks-model-provider-service: c.s.mps" in headers |
There was a problem hiding this comment.
why is this being lowercased
2 participants
Summary
--parent <catalog>.<schema>to Claude launchesTesting
/modelmatched all 14 models undermain.andye2e test with LiteSwap