Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add preliminary parsing of parameters #13

Merged
merged 1 commit into from
Oct 8, 2024
Merged

feat: add preliminary parsing of parameters #13

merged 1 commit into from
Oct 8, 2024

Conversation

kurtisvg
Copy link
Collaborator

@kurtisvg kurtisvg commented Oct 7, 2024

This PR adds preliminary parsing of parameters. Currently it only supports 4 types: string, int, float32, and bool. Almost certainly we will need to introduce more complicated parsing configuration (to handle objects and arrays), but my initial attempts got quickly complicated, so I simplified in the short term.

This also makes 2 breaking changes to config.yaml:

  • changes "parameters" to be a list over object -- this is because parameter ordering is important, and needs to be preserved
  • removed the "required" field from parameter objects -- we need to determine how to handle optional parameters in SQL queries

@kurtisvg kurtisvg requested review from Yuan325 and duwenxin99 October 7, 2024 21:19
@kurtisvg kurtisvg requested a review from a team as a code owner October 7, 2024 21:19
@kurtisvg kurtisvg force-pushed the kvg-params branch 2 times, most recently from 82bc245 to 187dbf1 Compare October 8, 2024 14:42
type Parameter struct {
Name string `yaml:"name"`
Type string `yaml:"type"`
Description string `yaml:"description"`
Required bool `yaml:"required"`
Copy link
Contributor

@Yuan325 Yuan325 Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note but I'm thinking having this might help improve latency too(?). Ideally, the LLM will prompt user to provide more information before moving on to the next steps since the field is required.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm removing this because I want to default for all parameters to be required -- I'm unsure of how to handle optional parameters in the SQL query

@Yuan325 Yuan325 self-requested a review October 8, 2024 18:18
@kurtisvg kurtisvg merged commit 27edd3b into main Oct 8, 2024
5 checks passed
@kurtisvg kurtisvg deleted the kvg-params branch October 8, 2024 21:18
kurtisvg pushed a commit that referenced this pull request Oct 28, 2024
🤖 I have created a release *beep* *boop*
---


## 0.0.1 (2024-10-28)


### Features

* Add address and port flags
([#7](#7))
([df9ad9e](df9ad9e))
* Add AlloyDB source and tool
([#23](#23))
([fe92d02](fe92d02))
* Add basic CLI
([#5](#5))
([1539ee5](1539ee5))
* Add basic http server
([#6](#6))
([e09ae30](e09ae30))
* Add basic parsing from tools file
([#8](#8))
([b9ba364](b9ba364))
* Add initial cloud sql pg invocation
([#14](#14))
([3703176](3703176))
* Add Postgres source and tool
([#25](#25))
([2742ed4](2742ed4))
* Add preliminary parsing of parameters
([#13](#13))
([27edd3b](27edd3b))
* Add support for array type parameters
([#26](#26))
([3903e86](3903e86))
* Add toolset configuration
([#12](#12))
([59b4bc0](59b4bc0))
* Add Toolset manifest endpoint
([#11](#11))
([61e7b78](61e7b78))
* **langchain-sdk:** Add Toolbox SDK for LangChain
([#22](#22))
([0bcd4b6](0bcd4b6))
* Stub basic control plane functionality
([#9](#9))
([336bdc4](336bdc4))


### Miscellaneous Chores

* Release 0.0.1
([#31](#31))
([1f24ddd](1f24ddd))


### Continuous Integration

* Add realease-please
([#15](#15))
([17fbbb4](17fbbb4))

---
This PR was generated with [Release
Please](/~https://github.com/googleapis/release-please). See
[documentation](/~https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Yuan325 pushed a commit that referenced this pull request Oct 29, 2024
🤖 I have created a release *beep* *boop*
---

* Add address and port flags
([#7](#7))
([df9ad9e](df9ad9e))
* Add AlloyDB source and tool
([#23](#23))
([fe92d02](fe92d02))
* Add basic CLI
([#5](#5))
([1539ee5](1539ee5))
* Add basic http server
([#6](#6))
([e09ae30](e09ae30))
* Add basic parsing from tools file
([#8](#8))
([b9ba364](b9ba364))
* Add initial cloud sql pg invocation
([#14](#14))
([3703176](3703176))
* Add Postgres source and tool
([#25](#25))
([2742ed4](2742ed4))
* Add preliminary parsing of parameters
([#13](#13))
([27edd3b](27edd3b))
* Add support for array type parameters
([#26](#26))
([3903e86](3903e86))
* Add toolset configuration
([#12](#12))
([59b4bc0](59b4bc0))
* Add Toolset manifest endpoint
([#11](#11))
([61e7b78](61e7b78))
* **langchain-sdk:** Add Toolbox SDK for LangChain
([#22](#22))
([0bcd4b6](0bcd4b6))
* Stub basic control plane functionality
([#9](#9))
([336bdc4](336bdc4))

* Release 0.0.1
([#31](#31))
([1f24ddd](1f24ddd))

* Add realease-please
([#15](#15))
([17fbbb4](17fbbb4))

---
This PR was generated with [Release
Please](/~https://github.com/googleapis/release-please). See
[documentation](/~https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants