#260: break datastore cycles via factories #268

tseaver · 2014-10-20T19:53:44Z

My take on breaking the import cycles in 'gcloud.datastore' by adding a "factory registry" for the classes normally imported.

Note that some classes have more than one factory (class methods like'from_protobuf' and 'from_path'): each is registered under a separate name.

Also drops 'Dataset.query' and 'Dataset.entity' convenience APIs, pointing instead to constructing Query and Entity directly.

Addresses #260 (for 'gcloud.datastore').

gcloud/datastore/__init__.py

+>>> from gcloud.datastore.entity import Entity
+>>> from gcloud.datastore.query import Query
+>>> query = Query('EntityKind', dataset)
+>>> entity = Entity(dataset, 'EntityKind')


coveralls · 2014-10-20T20:31:15Z

Coverage remained the same when pulling 6a77c1a on tseaver:260-break_datastore_cycles_via_factories into 99e4d53 on GoogleCloudPlatform:master.

coveralls · 2014-10-20T20:36:26Z

Coverage remained the same when pulling 6a77c1a on tseaver:260-break_datastore_cycles_via_factories into 99e4d53 on GoogleCloudPlatform:master.

gcloud/datastore/_helpers.py

+    _FACTORIES[name] = factory
+
+
+def _query_factory(name):


dhermes · 2014-10-20T22:30:37Z

I think this concept is a little too Pythonic.

In other words, cyclic imports are more desirable to me than a module global which is mutable.

Having _FACTORIES change at run time makes the codebase impossible to reason about statically as well.

tseaver · 2014-10-20T23:14:40Z

@dhermes I don't agree about the "mutable global": modules themselves get mutated during import. Note that the _FACTORIES bit (originally _factories, until pylint's bitching made me changed it) gets changed only as the registering module is imported (except for tests). Cyclical imports are a sign of much worse muddiness than a simple, controlled registry.

coveralls · 2014-10-20T23:22:36Z

Coverage remained the same when pulling cb2352f on tseaver:260-break_datastore_cycles_via_factories into 99e4d53 on GoogleCloudPlatform:master.

dhermes · 2014-10-21T01:11:47Z

Potential compromise:

Can we add very clear docs to _helpers and give references to all the places we expect a class to be registered?

dhermes · 2014-10-21T01:12:33Z

I also realized that the "run time" complaints are contradictory since a import inside a method/function happens at run time.

tseaver · 2014-10-21T04:12:09Z

1305e81 encapsulates the registry as an object with 'register', 'get', and 'invoke' methods. It also ensures that the relevant modules get imported early (to ensure that the registry is populated).

coveralls · 2014-10-21T04:22:56Z

Coverage remained the same when pulling 1305e81 on tseaver:260-break_datastore_cycles_via_factories into 99e4d53 on GoogleCloudPlatform:master.

gcloud/datastore/_helpers.py

+
+
+_FACTORIES = _FactoryRegistry()  # singleton
+del _FactoryRegistry


coveralls · 2014-10-21T17:22:54Z

Coverage remained the same when pulling fd9cd02 on tseaver:260-break_datastore_cycles_via_factories into 99e4d53 on GoogleCloudPlatform:master.

coveralls · 2014-10-21T17:28:58Z

Coverage remained the same when pulling fd9cd02 on tseaver:260-break_datastore_cycles_via_factories into 99e4d53 on GoogleCloudPlatform:master.

coveralls · 2014-10-22T18:34:00Z

Coverage remained the same when pulling ccc79fd on tseaver:260-break_datastore_cycles_via_factories into c2a38fb on GoogleCloudPlatform:master.

Point instead to constructing Query and Entity directly. Addresses #260, but only partially, for gcloud.datastore.

Work toward breaking cycles based on import-for-construction.

Note that some classes have more than one factory (class methodsd like 'from_protobuf' and 'from_path'). Each is registered under a separate name. Addresses #260 (for gcloud.datastore).

Incorporates feedback from @dhermees.

@dhermes

Incorporate feedback from @dhermes.

* docs: Fix formatting of request arg in docstring chore: Update gapic-generator-python to v1.9.1 PiperOrigin-RevId: 518604533 Source-Link: googleapis/googleapis@8a085ae Source-Link: googleapis/googleapis-gen@b2ab4b0 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYjJhYjRiMGEwYWUyOTA3ZTgxMmMyMDkxOThhNzRlMDg5OGFmY2IwNCJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>

- [ ] Regenerate this pull request now. PiperOrigin-RevId: 459095142 Source-Link: googleapis/googleapis@4f1be99 Source-Link: googleapis/googleapis-gen@ae686d9 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYWU2ODZkOWNkZTRmYzNlMzZkMGFjMDJlZmI4NjQzYjE1ODkwYzFlZCJ9 feat: add audience parameter PiperOrigin-RevId: 456827138 Source-Link: googleapis/googleapis@23f1a15 Source-Link: googleapis/googleapis-gen@4075a85 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNDA3NWE4NTE0ZjY3NjY5MWVjMTU2Njg4YTViYmYxODNhYTk4OTNjZSJ9

* docs: convert UPGRADING guide to RST to fix table formatting * use latest post processor image * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>

* docs: reverts the table addition from #114 Removed formatted tables (reverts #114) Converted file back to .MD (reverts part of #268) * docs: revert changes to link * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * docs: putting back this section Owlbot removed this but we need to keep it. * docs: fix whitespace issue * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * rename docs/UPGRADING.rst to docs/UPGRADING.md * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

Source-Link: googleapis/synthtool@703554a Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:94961fdc5c9ca6d13530a6a414a49d2f607203168215d074cdb0a1df9ec31c0b

Source-Link: https://github.com/googleapis/synthtool/commit/92006bb3cdc84677aa93c7f5235424ec2b157146 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:2e247c7bf5154df7f98cce087a20ca7605e236340c7d6d1a14447e5c06791bd6

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>

- [ ] Regenerate this pull request now. docs: clarified wording around Cloud Storage usage PiperOrigin-RevId: 433282831 Source-Link: googleapis/googleapis@0e87dc7 Source-Link: googleapis/googleapis-gen@1928631 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMTkyODYzMWYzZWU5MTQ4OGZlMDdiM2Q4MTg4YTUyZDEwODYyNDUzYSJ9

* chore: Update gapic-generator-python to v1.8.4 PiperOrigin-RevId: 507808936 Source-Link: googleapis/googleapis@64cf849 Source-Link: googleapis/googleapis-gen@53c48ca Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNTNjNDhjYWMxNTNkM2IzN2YzZDJjMmRlYzQ4MzBjZmQ5MWVjNDE1MyJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

#268)

chore: relocate owl bot post processor

…le (#268) Source-Link: googleapis/synthtool@a7ed11e Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:6e7328583be8edd3ba8f35311c76a1ecbc823010279ccb6ab46b7a76e25eafcc Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

* chore: Update gapic-generator-python to v1.8.5 PiperOrigin-RevId: 511892190 Source-Link: googleapis/googleapis@a45d9c0 Source-Link: googleapis/googleapis-gen@1907294 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMTkwNzI5NGIxZDgzNjVlYTI0ZjhjNWYyZTA1OWE2NDEyNGM0ZWQzYiJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>

fix(deps): require proto-plus >= 1.22.0

* chore: upgrade gapic-generator-java, gax-java and gapic-generator-python PiperOrigin-RevId: 423842556 Source-Link: googleapis/googleapis@a616ca0 Source-Link: googleapis/googleapis-gen@29b938c Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMjliOTM4YzU4YzFlNTFkMDE5ZjJlZTUzOWQ1NWRjMGEzYzg2YTkwNSJ9 * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

#268)

Source-Link: googleapis/synthtool@f15cc72 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:bc5eed3804aec2f05fad42aacf973821d9500c174015341f721a984a0825b6fd

Source-Link: googleapis/synthtool@993985f Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:1894490910e891a385484514b22eb5133578897eb5b3c380e6d8ad475c6647cd

* ci(python): run lint / unit tests / docs as GH actions Source-Link: googleapis/synthtool@57be0cd Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:ed1f9983d5a935a89fe8085e8bb97d94e41015252c5b6c9771257cf8624367e6 * add commit to trigger gh actions * work around template bug Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

Source-Link: https://github.com/googleapis/synthtool/commit/d0f51a0c2a9a6bcca86911eabea9e484baadf64b Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:240b5bcc2bafd450912d2da2be15e62bc6de2cf839823ae4bf94d4f392b451dc

@busunkim96

- [ ] Regenerate this pull request now. chore: fix docstring for first attribute of protos committer: @busunkim96 PiperOrigin-RevId: 401271153 Source-Link: googleapis/googleapis@787f8c9 Source-Link: googleapis/googleapis-gen@81decff Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiODFkZWNmZmU5ZmM3MjM5NmE4MTUzZTc1NmQxZDY3YTZlZWNmZDYyMCJ9

Source-Link: googleapis/synthtool@50db768 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:e09366bdf0fd9c8976592988390b24d53583dd9f002d476934da43725adbb978

Source-Link: https://github.com/googleapis/synthtool/commit/352b9d4c068ce7c05908172af128b294073bf53c Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:3e3800bb100af5d7f9e810d48212b37812c1856d20ffeafb99ebe66461b61fc7

tseaver added api: datastore Issues related to the Datastore API. hygiene labels Oct 20, 2014

dhermes reviewed Oct 20, 2014
View reviewed changes

tseaver mentioned this pull request Oct 20, 2014

Disentangle import cycles #260

Closed

dhermes reviewed Oct 20, 2014
View reviewed changes

gcloud/datastore/_helpers.py

_FACTORIES[name] = factory

def _query_factory(name):

This comment was marked as spam.

Sign in to view

dhermes reviewed Oct 21, 2014
View reviewed changes

gcloud/datastore/_helpers.py

_FACTORIES = _FactoryRegistry() # singleton

del _FactoryRegistry

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

tseaver added 8 commits October 22, 2014 15:34

Drop 'Dataset.query' and 'Dataset.entity' convenience APIs.

1036f85

Point instead to constructing Query and Entity directly. Addresses #260, but only partially, for gcloud.datastore.

Add a factory registry to datastore._helpers.

1c432a9

Work toward breaking cycles based on import-for-construction.

Break import cycles via a factory registry.

8d05122

Note that some classes have more than one factory (class methodsd like 'from_protobuf' and 'from_path'). Each is registered under a separate name. Addresses #260 (for gcloud.datastore).

Tidy local imports, remove remaining cycles.

3c1d516

Fix build: invoke correct factory when unmarshalling key from pb.

cf29c90

Reorder 'Query.__init__' paramters to match Entity.

31f60d8

Rename '_query_factory' helper -> '_get_factory' to avoid confustion.

3a62db9

Incorporates feedback from @dhermees.

Avoid relative imports.

daf889c

Incorporate feedback from @dhermes.

parthea added a commit that referenced this pull request Jun 4, 2023

chore(main): release 1.3.0 (#268)

8368054

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>

parthea pushed a commit that referenced this pull request Jun 4, 2023

chore(main): release 1.4.0 (#268)

814e712

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

parthea added a commit that referenced this pull request Jun 4, 2023

chore(main): release 2.9.2 (#268)

a528192

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>

parthea pushed a commit that referenced this pull request Sep 20, 2023

chore(deps): update dependency google-cloud-video-transcoder to v1.6.0 (

06fa9b4

#268)

parthea pushed a commit that referenced this pull request Sep 22, 2023

build: switch to release-please for tagging (#268)

ed5ed6d

parthea pushed a commit that referenced this pull request Sep 22, 2023

chore: relocate owl bot post processor (#268)

87db1f9

chore: relocate owl bot post processor

parthea added a commit that referenced this pull request Sep 22, 2023

fix(deps): allow protobuf < 5.0.0 (#268)

e2d77e0

fix(deps): require proto-plus >= 1.22.0

parthea pushed a commit that referenced this pull request Oct 21, 2023

chore(deps): update dependency google-cloud-resource-manager to v1.1.0 (

51bb6f9

#268)

parthea pushed a commit that referenced this pull request Oct 21, 2023

chore: release 3.5.0 (#268)

e480db3

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

parthea pushed a commit that referenced this pull request Oct 21, 2023

chore(deps): update dependency google-analytics-admin to v0.11.0 (#268)

c755066

parthea pushed a commit that referenced this pull request Oct 21, 2023

chore(quickstart): clarify private pools instructions (#268)

c575a48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#260: break datastore cycles via factories #268

#260: break datastore cycles via factories #268

tseaver commented Oct 20, 2014

This comment was marked as spam.

This comment was marked as spam.

coveralls commented Oct 20, 2014

coveralls commented Oct 20, 2014

This comment was marked as spam.

dhermes commented Oct 20, 2014

tseaver commented Oct 20, 2014

coveralls commented Oct 20, 2014

dhermes commented Oct 21, 2014

dhermes commented Oct 21, 2014

tseaver commented Oct 21, 2014

coveralls commented Oct 21, 2014

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

coveralls commented Oct 21, 2014

coveralls commented Oct 21, 2014

coveralls commented Oct 22, 2014



		_FACTORIES = _FactoryRegistry() # singleton
		del _FactoryRegistry

#260: break datastore cycles via factories #268

#260: break datastore cycles via factories #268

Conversation

tseaver commented Oct 20, 2014

This comment was marked as spam.

This comment was marked as spam.

coveralls commented Oct 20, 2014

coveralls commented Oct 20, 2014

This comment was marked as spam.

dhermes commented Oct 20, 2014

tseaver commented Oct 20, 2014

coveralls commented Oct 20, 2014

dhermes commented Oct 21, 2014

dhermes commented Oct 21, 2014

tseaver commented Oct 21, 2014

coveralls commented Oct 21, 2014

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

coveralls commented Oct 21, 2014

coveralls commented Oct 21, 2014

coveralls commented Oct 22, 2014