Skip to content

Fix cookie setting #379

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

Merged
merged 2 commits into from
Mar 27, 2024
Merged

Fix cookie setting #379

merged 2 commits into from
Mar 27, 2024

Conversation

benc-db
Copy link
Collaborator

@benc-db benc-db commented Mar 26, 2024

Small revamp of PR that @susodapop put out in October. I suspect this might be breaking now (see databricks/dbt-databricks#622)

Also, I removed a test that happens to still pass successfully, even though it shouldn't pass after my previous change to retry behavior. For whatever reason, the mocking of the status code is not making it into the retry logic.

Signed-off-by: Ben Cassell <[email protected]>
@kravets-levko
Copy link
Contributor

kravets-levko commented Mar 26, 2024

@benc-db there is a task to remove cookies handling at all - PECO-1064. We already removed it in Node connector a while ago, and IIRC Go driver didn't even have similar code. Do you know any use cases where cookies may be needed in PySQL?

@benc-db
Copy link
Collaborator Author

benc-db commented Mar 26, 2024

@benc-db there is a task to remove cookies handling at all - PECO-1064. We already removed it in Node connector a while ago, and IIRC Go driver didn't even have similar code. Do you know any use cases where cookies may be needed in PySQL?

If we can just delete it, I'll go ahead and do that.

Copy link

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Signed-off-by: Ben Cassell <[email protected]>
@benc-db benc-db merged commit 7fade51 into main Mar 27, 2024
13 checks 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.

2 participants