Skip to content

pass settings in all HTTP provider invocations#210

Merged
0xnmn merged 1 commit intomainfrom
sqs/http-pass-settings
Oct 25, 2024
Merged

pass settings in all HTTP provider invocations#210
0xnmn merged 1 commit intomainfrom
sqs/http-pass-settings

Conversation

@sqs
Copy link
Copy Markdown
Member

@sqs sqs commented Oct 25, 2024

The spec says that HTTP providers are invoked with a POST whose body includes the settings (https://openctx.org/docs/protocol). However, this was not happening; only the params were being sent. This fix makes it so that the settings are also passed.

The spec says that HTTP providers are invoked with a POST whose body includes the settings (https://openctx.org/docs/protocol). However, this was not happening; only the `params` were being sent. This fix makes it so that the `settings` are also passed.
@sqs sqs requested a review from 0xnmn October 25, 2024 08:09
Copy link
Copy Markdown
Contributor

@0xnmn 0xnmn left a comment

Choose a reason for hiding this comment

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

LGTM. Do we have any user using a HTTP provider btw?

@0xnmn 0xnmn merged commit ffa9763 into main Oct 25, 2024
@0xnmn 0xnmn deleted the sqs/http-pass-settings branch October 25, 2024 17:27
@0xnmn
Copy link
Copy Markdown
Contributor

0xnmn commented Oct 25, 2024

@sqs this would need a followup version bump and publish. I am out right now, so can't do it.

sqs added a commit to sourcegraph/cody-public-snapshot that referenced this pull request Oct 28, 2024
sqs added a commit to sourcegraph/cody-public-snapshot that referenced this pull request Oct 28, 2024
bevzzz pushed a commit to bevzzz/openctx that referenced this pull request Dec 12, 2024
The spec says that HTTP providers are invoked with a POST whose body
includes the settings (https://openctx.org/docs/protocol). However, this
was not happening; only the `params` were being sent. This fix makes it
so that the `settings` are also passed.
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