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

Increase timeout for querying snapshots table #544

Merged
merged 1 commit into from
Aug 31, 2023

Conversation

tomach
Copy link
Contributor

@tomach tomach commented Aug 29, 2023

Summary of changes

Fixes https://github.com/crate/cloud/issues/1362

Checklist

  • Relevant changes are reflected in CHANGES.rst
  • Added or changed code is covered by tests
  • Documentation has been updated if necessary
  • Changed code does not contain any breaking changes (or this is a major version change)

@@ -871,16 +879,15 @@ async def handle( # type: ignore
get_host(core, namespace, name),
)
conn_factory = connection_factory(host, password)
async with conn_factory() as conn:
async with conn.cursor() as cursor:
Copy link
Contributor

Choose a reason for hiding this comment

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

Could have also set the timeout globally here perhaps? Would have made a smaller diff! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, but then the timeout would be increased for everything in validate_restore_complete() like querying sys.shards etc. not sure if we want that?

The diff should become smaller once I merge the other branch into master - I just needed that code as a base here :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it matters that much, the one minute timeout is a bit arbitrary, same as 2 minutes! Up to you entirely, I already approved it.

@tomach tomach marked this pull request as ready for review August 31, 2023 14:51
@tomach tomach merged commit bf89d59 into master Aug 31, 2023
5 checks passed
@tomach tomach deleted the ta/fix-snapshots-timeout branch August 31, 2023 14:52
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