Skip to content

Conversation

chemelnucfin
Copy link
Contributor

No description provided.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 8, 2017
@chemelnucfin chemelnucfin added the api: spanner Issues related to the Spanner API. label Dec 20, 2017
@tseaver
Copy link
Contributor

tseaver commented Dec 20, 2017

This change is not correct. We are forced to re-use / share instances across test runs (and for other purposes), which means that we should only ever drop databases which are created in the current test run.

@tseaver tseaver closed this Dec 20, 2017
@chemelnucfin
Copy link
Contributor Author

chemelnucfin commented Dec 20, 2017 via email

@dhermes
Copy link
Contributor

dhermes commented Dec 20, 2017

especially if I interrupt in the middle

Our cleanup has been tailored to a "graceful" exit (i.e. tests may error, but user doesn't kill process).

We could consider a more resilient approach but I don't think it's worth the effort.

@tseaver
Copy link
Contributor

tseaver commented Dec 20, 2017

Until we go back to using a created-from-scratch instance, we cannot drop all databases, because the CI may be running more than one job at once in the same instance. There may also be other databases pre-populated (e.g., to run the "large chunk" tests) which should never be dropped.

vchudnov-g pushed a commit that referenced this pull request Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants