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

Enables Copilot to send cookies when calling Chainlit server #1339

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mattgartman
Copy link

Enables the Copilot functionality to send cookies with requests to the Chainlit server. This allows support for cookie based sticky sessions.

Since withCredentials does not work when a server sets a wildcard CORS origin policy, this update exposes an operational parameter for users needing cookies to be sent with requests.

This PR addresses #1279.

@dokterbob
Copy link
Collaborator

@dosu Could you please add appropriate labels to this PR?

@dokterbob
Copy link
Collaborator

Sorry it took us a while to get back to this.

I just found this related PR, it seems it's covering the same scope -- it just misses a minimal update to useChatSession. #1497

Perhaps there's the opportunity for collaboration to solve these issues in one go? @nethi @mattgartman @ShubhamMaddhashiya-bidgely

@dokterbob dokterbob added the blocked Awaiting update or feedback from user after initial review/comments. label Nov 6, 2024
@ShubhamMaddhashiya-bidgely

@dokterbob In this PR, along with changes in libs/react-client/src/useChatSession.ts, there are other changes in copilot-related files to pass the send cookies bool from top to socket client.

We would need similar changes for chainlit app (webapp) side right?

Perhaps there's the opportunity for collaboration to solve these issues in one go?

are you proposing we combine these two PRs?

@dokterbob
Copy link
Collaborator

@dokterbob In this PR, along with changes in libs/react-client/src/useChatSession.ts, there are other changes in copilot-related files to pass the send cookies bool from top to socket client.

We would need similar changes for chainlit app (webapp) side right?

Perhaps there's the opportunity for collaboration to solve these issues in one go?

are you proposing we combine these two PRs?

Yes and/or help review each other's work.

@ShubhamMaddhashiya-bidgely

Hey @mattgartman, what do you think about introducing a generic configuration object that can be passed to the SocketIO client instead of adding each parameter individually? Alternatively, we could allow passing a pre-configured client instance directly from upstream methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Awaiting update or feedback from user after initial review/comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants