changing schema naming #100
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alexagriffith
force-pushed
the
agriffith/rename-schema-props
branch
2 times, most recently
from
69d82c8 to
bf0151d
Compare
Signed-off-by: Alexa Griffith <[email protected]>
Signed-off-by: Alexa Griffith <[email protected]>
Signed-off-by: Alexa Griffith <[email protected]>
alexagriffith
force-pushed
the
agriffith/rename-schema-props
branch
from
a297206 to
d8b94d0
Compare
Signed-off-by: Alexa Griffith <[email protected]>
|
Could you keep the PR draft until all CI happy? |
mathetake
reviewed
Jan 15, 2025
alexagriffith
force-pushed
the
agriffith/rename-schema-props
branch
2 times, most recently
from
9ba2a3a to
068ce0e
Compare
Signed-off-by: Alexa Griffith <[email protected]> change nested schema to name in structs Signed-off-by: Alexa Griffith <[email protected]> fix factory logic Signed-off-by: Alexa Griffith <[email protected]> rebase# This is a combination of 20 commits. fix some other schema names to be consistent Signed-off-by: Alexa Griffith <[email protected]> gitignore idea Signed-off-by: Alexa Griffith <[email protected]> fix missing name change Signed-off-by: Alexa Griffith <[email protected]> fix filterconfig Signed-off-by: Alexa Griffith <[email protected]> fix test Signed-off-by: Alexa Griffith <[email protected]> renaming Signed-off-by: Alexa Griffith <[email protected]> fix Signed-off-by: Alexa Griffith <[email protected]> fix sink test naming Signed-off-by: Alexa Griffith <[email protected]> fix controller test Signed-off-by: Alexa Griffith <[email protected]> add name rule Signed-off-by: Alexa Griffith <[email protected]> fix rule Signed-off-by: Alexa Griffith <[email protected]> rename klog pkg Signed-off-by: Alexa Griffith <[email protected]> fix cel tests Signed-off-by: Alexa Griffith <[email protected]> re-add openai pkg Signed-off-by: Alexa Griffith <[email protected]> fix llm backend yaml Signed-off-by: Alexa Griffith <[email protected]> fix backend Signed-off-by: Alexa Griffith <[email protected]> fix the code style Signed-off-by: Alexa Griffith <[email protected]> fix order Signed-off-by: Alexa Griffith <[email protected]> fix Signed-off-by: Alexa Griffith <[email protected]> lint Signed-off-by: Alexa Griffith <[email protected]>
alexagriffith
force-pushed
the
agriffith/rename-schema-props
branch
from
068ce0e to
2e997d4
Compare
yuzisun
reviewed
Jan 15, 2025
| // InputSchema is a required field | ||
| InputSchema *ToolInputSchema `json:"inputSchema"` | ||
| // Schema is a required field | ||
| Schema *ToolSchema `json:"schema"` |
There was a problem hiding this comment.
This should not be changed, it is the AWS bedrock schema
There was a problem hiding this comment.
offline discussion - fixed this. didn't realize this was a field required by aws and that their are issues using their api for these types.
There was a problem hiding this comment.
note: I think we should have a test for if our hard coded fields and theirs misalign
Signed-off-by: Alexa Griffith <[email protected]>
yuzisun
reviewed
Jan 15, 2025
mathetake
added a commit
that referenced
this pull request
Jan 16, 2025
Signed-off-by: Takeshi Yoneda <[email protected]>
Krishanx92
reviewed
Jan 16, 2025
| // APISchema corresponds to APISchema in api/v1alpha1/api.go. | ||
| type APISchema string | ||
| // APISchemaName corresponds to APISchemaName in api/v1alpha1/api.go. | ||
| type APISchemaName string |
There was a problem hiding this comment.
Given that the values are provider names, wouldn't APISchemaProviderName be more suitable than APISchemaName?
There was a problem hiding this comment.
I dont have a big preference, open for discussion on this
Signed-off-by: Alexa Griffith <[email protected]>
|
@alexagriffith need to rebase your PR |
Signed-off-by: Alexa Griffith <[email protected]>
Signed-off-by: Alexa Griffith <[email protected]>
Signed-off-by: Alexa Griffith <[email protected]>
Signed-off-by: Alexa Griffith <[email protected]>
Signed-off-by: Alexa Griffith <[email protected]>
Signed-off-by: Alexa Griffith <[email protected]>
Signed-off-by: Alexa Griffith <[email protected]>
Signed-off-by: Alexa Griffith <[email protected]>
Signed-off-by: Alexa Griffith <[email protected]>
Signed-off-by: Alexa Griffith <[email protected]>
alexagriffith
force-pushed
the
agriffith/rename-schema-props
branch
from
309fb40 to
2215b9f
Compare
Signed-off-by: Alexa Griffith <[email protected]>
Signed-off-by: Alexa Griffith <[email protected]>
yuzisun
reviewed
Jan 17, 2025
internal/extproc/watcher_test.go
Outdated
| schema: OpenAI | ||
| selectedBackendHeaderKey: x-envoy-ai-gateway-selected-backend | ||
| modelNameHeaderKey: x-model-name | ||
| rules: | ||
| - backends: | ||
| - name: kserve | ||
| weight: 1 | ||
| outputSchema: | ||
| schema: | ||
| schema: OpenAI |
There was a problem hiding this comment.
Need to update to name now
There was a problem hiding this comment.
wonder why the CI did not complain
yuzisun
reviewed
Jan 17, 2025
internal/extproc/watcher_test.go
Outdated
| schema: OpenAI | ||
| - name: awsbedrock | ||
| weight: 10 | ||
| outputSchema: | ||
| schema: | ||
| schema: AWSBedrock |
yuzisun
reviewed
Jan 17, 2025
internal/extproc/watcher_test.go
Outdated
| schema: AWSBedrock | ||
| headers: | ||
| - name: x-model-name | ||
| value: llama3.3333 | ||
| - backends: | ||
| - name: openai | ||
| outputSchema: | ||
| schema: | ||
| schema: OpenAI |
yuzisun
reviewed
Jan 18, 2025
yuzisun
reviewed
Jan 18, 2025
yuzisun
reviewed
Jan 18, 2025
Signed-off-by: Dan Sun <[email protected]>
yuzisun
reviewed
Jan 18, 2025
yuzisun
reviewed
Jan 18, 2025
yuzisun
reviewed
Jan 18, 2025
yuzisun
reviewed
Jan 18, 2025
Signed-off-by: Dan Sun <[email protected]>
yuzisun
approved these changes
Jan 18, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#89
changing all instances of inputSchema and outputSchema to
schemafor simplicity.once that was done, the yaml file had a nested structure of schema.schema - schema can have two properties in it, schema and version. this is a bit redundant so schema.schema was changed to schema.name since the value is the name of the schema for a given versioned schema struct