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

fix(firestore): Cleanup integration test resources #8057

Merged
merged 3 commits into from Jun 12, 2023

Conversation

bhshkh
Copy link
Contributor

@bhshkh bhshkh commented Jun 6, 2023

Fixing #7998
Builds are failing with below error because composite indexes weren't getting cleared:
"You have reached the maximum number of indexes per database. Please delete an unused index and try again."

@bhshkh bhshkh requested a review from codyoss June 6, 2023 10:11
@bhshkh bhshkh requested review from a team as code owners June 6, 2023 10:11
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: datastore Issues related to the Datastore API. labels Jun 6, 2023
@bhshkh bhshkh requested a review from enocom June 6, 2023 17:26
@bhshkh bhshkh mentioned this pull request Jun 6, 2023
Copy link
Member

@enocom enocom left a comment

Choose a reason for hiding this comment

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

Two minor comments. Big picture LGTM

firestore/integration_test.go Outdated Show resolved Hide resolved
}
}
}

func cleanupIntegrationTest() {
if iClient != nil {
// TODO(jba): delete everything in integrationColl.
Copy link
Member

Choose a reason for hiding this comment

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

Does this TODO have any significance? If not, I'd suggest deleting it since jba no longer works on this project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed todo. Added cleanup of collection

Copy link
Member

@enocom enocom left a comment

Choose a reason for hiding this comment

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

Two minor comments. Big picture LGMT

Copy link
Member

@enocom enocom left a comment

Choose a reason for hiding this comment

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

Two minor comments. Big picture LGMT

@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Jun 10, 2023
@bhshkh bhshkh requested a review from enocom June 10, 2023 00:53
@bhshkh bhshkh changed the title fix(datastore): Cleanup composite indexes fix(firestore): Cleanup integration test resources Jun 10, 2023
@bhshkh bhshkh enabled auto-merge (squash) June 10, 2023 02:38
@product-auto-label product-auto-label bot added api: firestore Issues related to the Firestore API. and removed api: datastore Issues related to the Datastore API. labels Jun 10, 2023
@noahdietz noahdietz added the automerge Merge the pull request once unit tests and other checks pass. label Jun 12, 2023
@noahdietz
Copy link
Contributor

@bhshkh did you need another approval from @enocom ? or is this good to go? (i'm on rotation right now so just going through PRs)

@enocom
Copy link
Member

enocom commented Jun 12, 2023

This is good to go in my view.

@bhshkh bhshkh merged commit 210584d into googleapis:main Jun 12, 2023
8 checks passed
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jun 12, 2023
xwkuang5 pushed a commit to xwkuang5/google-cloud-go that referenced this pull request Jun 13, 2023
…nto agg-add-trace

* 'agg-add-trace' of github.com:xwkuang5/google-cloud-go:
  Fix formating.
  chore(firestore): Add bloom filter related comments (googleapis#8079)
  fix(firestore): Cleanup integration test resources (googleapis#8057)
  fix(datastore): Handling nil slices in save and query (googleapis#8043)
@bhshkh bhshkh deleted the issue-7998 branch July 18, 2023 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the Firestore API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants