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

add lint: containedctx #1240

Merged
merged 2 commits into from
Feb 10, 2024
Merged

add lint: containedctx #1240

merged 2 commits into from
Feb 10, 2024

Conversation

serprex
Copy link
Contributor

@serprex serprex commented Feb 9, 2024

Required updating connector interfaces. Bit annoying ctx everywhere, but that's ultimately the correct way. Was running into context complications in #1211 with connector being shared between activities

Putting context in struct essentially makes that struct a context, but this is not the context we necessarily want. For more context, see https://zenhorace.dev/blog/context-control-go

Some changes were made:

  1. GetCatalog takes a context now instead of using context.Background()
  2. eventhubs processBatch now takes context instead of using context.Background()
  3. many instances of Query/Exec in snowflake/clickhouse converted to QueryContext/ExecContext
  4. got rid of cancel context in ssh tunnel, context being passed in is sufficient

Followup to #1238

@iskakaushik iskakaushik merged commit 1213f44 into main Feb 10, 2024
7 checks passed
@iskakaushik iskakaushik deleted the containedctx branch February 10, 2024 15:47
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