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

feat: Multi db test parametrization #436

Merged
merged 14 commits into from
Jun 21, 2023

Conversation

Mariatta
Copy link
Contributor

@Mariatta Mariatta commented May 9, 2023

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

This PR adds changes on top of #398, which was not yet merged to main.

The changes are:

  • adding test parametrization wherever the client is used. It will run the test against the default db, and a named db
  • named db can only be passed to the Client, and the underlying queries and aggregation queries will retrieve the database id from the Client.
  • If the database is default, the request will not pass any database_id values
  • If the database name is specified, the request will pass a database_id field

Fixes #<issue_number_goes_here> 🦕

@Mariatta Mariatta requested review from a team as code owners May 9, 2023 05:19
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. api: datastore Issues related to the googleapis/python-datastore API. labels May 9, 2023
@Mariatta
Copy link
Contributor Author

Mariatta commented May 9, 2023

The systems tests are failing because we don't have the system-tests-named-db on the project used by CI.

Once the database created, populated with data, and indexes created, the tests should pass.

@kolea2 kolea2 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 9, 2023
@Mariatta Mariatta changed the base branch from main to multi-db May 9, 2023 20:11
@Mariatta Mariatta requested a review from a team as a code owner May 9, 2023 20:11
@Mariatta Mariatta requested review from atulep and removed request for a team May 9, 2023 20:11
@Mariatta Mariatta changed the base branch from multi-db to main May 9, 2023 20:11
@Mariatta Mariatta changed the base branch from main to preview-multidb May 9, 2023 20:15
Remove "database" argument from Query and AggregationQuery constructors.
Use the "database" from the client instead.
Once set in the client, the "database" will be used throughout and cannot be re-set.

Parametrize the tests where-ever clients are used.

Use the `system-tests-named-db` in the system test.
@Mariatta Mariatta force-pushed the feat-multi-db-test-parametrization branch from 513e772 to 544dab7 Compare May 9, 2023 20:27
@Mariatta
Copy link
Contributor Author

Mariatta commented May 9, 2023

This PR is now rebased against preview-multidb branch.

@Mariatta
Copy link
Contributor Author

Mariatta commented May 9, 2023

Systems test are now passing.

@kolea2 kolea2 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 6, 2023
@vishwarajanand
Copy link
Contributor

I've updated these PRs and got the tests green. Seeking a go ahead for merging this.

@meredithslota meredithslota removed the request for review from atulep June 14, 2023 17:36
@meredithslota
Copy link
Contributor

Removed a review request from Aza due to team transfer, but still seeking approval from a Python person.

google/cloud/datastore/helpers.py Outdated Show resolved Hide resolved
@m-strzelczyk m-strzelczyk merged commit 5499761 into preview-multidb Jun 21, 2023
5 checks passed
@m-strzelczyk m-strzelczyk deleted the feat-multi-db-test-parametrization branch June 21, 2023 10:34
m-strzelczyk pushed a commit that referenced this pull request Jun 21, 2023
* feat: named database support (#398)

* feat: Add named database support

* test: Use named db in system tests

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* Handle the case when client doesn't have database property

* fix: add custom routing headers

* Fixing tests for easier merge

* fixing code coverage

* addressing pr comments

* feat: Multi db test parametrization (#436)

* Feat: Parametrize the tests for multidb support

Remove "database" argument from Query and AggregationQuery constructors.
Use the "database" from the client instead.
Once set in the client, the "database" will be used throughout and cannot be re-set.

Parametrize the tests where-ever clients are used.

Use the `system-tests-named-db` in the system test.

* Add test case for when parent database name != child database name

* Update owlbot, removing the named db parameter

* Reverted test fixes

* fixing tests

* fix code coverage

* pr suggestion

* address pr comments

---------

Co-authored-by: Vishwaraj Anand <vishwaraj.anand00@gmail.com>

---------

Co-authored-by: Bob "Wombat" Hogg <wombat@rwhogg.site>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Vishwaraj Anand <vishwaraj.anand00@gmail.com>
Co-authored-by: meredithslota <meredithslota@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the googleapis/python-datastore API. size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants