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: named database support #439

Merged
merged 10 commits into from Jun 21, 2023
Merged

feat: named database support #439

merged 10 commits into from Jun 21, 2023

Conversation

Mariatta
Copy link
Contributor

@Mariatta Mariatta commented May 9, 2023

The branch multi-db was outdated. This PR makes it up to date with main and contains the commit from #398.

We will create the multi db test parametrization PR (#436) branch against this branch instead of main.

Once the feature is out of preview, we should:

* feat: Add named database support

* test: Use named db in system tests
@Mariatta Mariatta requested review from a team as code owners May 9, 2023 20:15
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: datastore Issues related to the googleapis/python-datastore API. labels May 9, 2023
@Mariatta
Copy link
Contributor Author

The http test is failing here, but it gets fixed by #436. So once we merge #436 into this branch, everything will be good.

@vishwarajanand
Copy link
Contributor

vishwarajanand commented Jun 13, 2023

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

Copy link
Contributor

@kolea2 kolea2 left a comment

Choose a reason for hiding this comment

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

LGTM after nits and pending approval from @googleapis/yoshi-python

google/cloud/datastore/_http.py Show resolved Hide resolved
google/cloud/datastore/constants.py Outdated Show resolved Hide resolved
@parthea parthea changed the title feat: named database support (#398) feat: named database support Jun 20, 2023
@parthea parthea added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 20, 2023
@parthea
Copy link
Contributor

parthea commented Jun 20, 2023

Wait for #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>
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Jun 21, 2023
@m-strzelczyk m-strzelczyk removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 21, 2023
@m-strzelczyk m-strzelczyk merged commit abf0060 into main Jun 21, 2023
16 checks passed
@m-strzelczyk m-strzelczyk deleted the preview-multidb branch June 21, 2023 10:36
@smittysmee
Copy link

smittysmee commented Jul 7, 2023

@Mariatta - how sure are we on regression here? (Support case now open)

Unhandled exception: 'Key' object has no attribute '_database'
...
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/ndb/_options.py", line 102, in wrapper
    return wrapped(*pass_args, **kwargs)
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/ndb/utils.py", line 118, in wrapper
    return wrapped(*args, **new_kwargs)
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/ndb/utils.py", line 150, in positional_wrapper
    return wrapped(*args, **kwds)
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/ndb/model.py", line 5389, in _put
    return self._put_async(_options=kwargs["_options"]).result()
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/ndb/tasklets.py", line 210, in result
    self.check_success()
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/ndb/tasklets.py", line 157, in check_success
    raise self._exception
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/ndb/tasklets.py", line 319, in _advance_tasklet
    yielded = self.generator.throw(type(error), error, traceback)
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/ndb/model.py", line 5450, in put
    ds_key = yield _datastore_api.put(ds_entity, kwargs["_options"])
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/ndb/tasklets.py", line 323, in _advance_tasklet
    yielded = self.generator.send(send_value)
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/ndb/_datastore_api.py", line 371, in put
    entity_pb = helpers.entity_to_protobuf(entity)
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/datastore/helpers.py", line 216, in entity_to_protobuf
    _set_protobuf_value(value_pb, value)
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/datastore/helpers.py", line 476, in _set_protobuf_value
    entity_pb = entity_to_protobuf(val)
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/datastore/helpers.py", line 216, in entity_to_protobuf
    _set_protobuf_value(value_pb, value)
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/datastore/helpers.py", line 470, in _set_protobuf_value
    attr, val = _pb_attr_value(val)
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/datastore/helpers.py", line 360, in _pb_attr_value
    name, value = "key", val.to_protobuf()
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/datastore/key.py", line 310, in to_protobuf
    if self.database:
  File "/layers/google.python.pip/pip/lib/python3.7/site-packages/google/cloud/datastore/key.py", line 421, in database
    return self._database
AttributeError: 'Key' object has no attribute '_database'

@parthea
Copy link
Contributor

parthea commented Jul 7, 2023

Hi @smittysmee,

Please could you file a bug with sample code? Please include the versions of google-cloud-ndb and google-cloud-datastore in the bug to help us re-create the issue.

@smittysmee
Copy link

smittysmee commented Jul 10, 2023

@parthea - it was a lack of backwards compatibility in a rollover and cache.

  • Previous version - get entity - update cache with entity A.
  • Released new version - cache hit of entity A - perform creation of entity B with reference to entity A, error.

Entity A did not update with latest key structure containing added attribute _database

@vishwarajanand
Copy link
Contributor

@smittysmee by cache hit, it suggests to me that the issue is local to your environment? Pls confirm if that was not the case.
Any sample to reproduce the issue like Parth suggested will be superhelpful.

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

8 participants