Skip to content

Define __all__ in gcloud/monitoring/__init__.py. #1747

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.

Sign up for GitHub

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

Jump to bottom
Merged
dhermes merged 1 commit into from
Apr 25, 2016
Merged

Define __all__ in gcloud/monitoring/__init__.py. #1747

dhermes merged 1 commit into from
Apr 25, 2016

Conversation

@rimey
Copy link
Contributor

@rimey rimey commented Apr 23, 2016

This is so that "from gcloud.monitoring import *" doesn't import
the package's module names.

@rimey
Define __all__ in gcloud/monitoring/__init__.py.
e46d81c 
This is so that "from gcloud.monitoring import *" doesn't import
the package's module names.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 23, 2016
@rimey rimey added the api: monitoring Issues related to the Cloud Monitoring API. label Apr 23, 2016
@tseaver
Copy link
Contributor

tseaver commented Apr 23, 2016

-0, mostly because from foo import * is a truly evil pattern. I would definitely vote to reject any docs change which showed from gcloud.monitoring import *.

@tseaver
Copy link
Contributor

tseaver commented Apr 23, 2016

Looking at the module: there are no names present which are not listed in __all__.

@rimey
Copy link
Contributor Author

rimey commented Apr 23, 2016

This is a package, so it brings in all the module names: client, connection, label, metric, ...

@rimey
Copy link
Contributor Author

rimey commented Apr 23, 2016

from gcloud.monitoring import * is useful interactively.

@rimey
Copy link
Contributor Author

rimey commented Apr 23, 2016 

>>> from gcloud import monitoring
>>> client = monitoring.Client('my-project')
>>> q = client.query(hours=2).align(Aligner.ALIGN_MEAN, minutes=5)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'Aligner' is not defined
>>> from gcloud.monitoring import *
>>> q = client.query(hours=2).align(Aligner.ALIGN_MEAN, minutes=5)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'module' object has no attribute 'query'

@dhermes
Copy link
Contributor

dhermes commented Apr 23, 2016

I'm fine with defining __all__. But, @jonparrott recently pointed out that imported modules are not considered part of the public API. From PEP8

Imported names should always be considered an implementation detail. Other modules must not rely on indirect access to such imported names unless they are an explicitly documented part of the containing module's API, such as os.path or a package's init module that exposes functionality from submodules.

For an example from the stdlib.

>>> import subprocess
>>> subprocess.sys
<module 'sys' (built-in)>

@jgeewax
Copy link
Contributor

jgeewax commented Apr 23, 2016

I'm OK with this change -- though I'm -1 to using the import * pattern in our documentation.

If people want to use that interactively that's fine with me, but it's certainly not recommended.

@dhermes
Copy link
Contributor

dhermes commented Apr 23, 2016

Shall we merge then? @tseaver any objections?

Ditto on that. Namespaces are one of Python's strengths (both in language design and community / "idiomatic" behavior). From the Zen of Python:

Namespaces are one honking great idea -- let's do more of those!

@rimey
Copy link
Contributor Author

rimey commented Apr 23, 2016

Defining __all__ in this circumstance is standard practice. You just don't see it that much outside of carefully curated libraries like the Python standard library, because it's of minor consequence, for the reasons you have mentioned.

@tseaver
Copy link
Contributor

tseaver commented Apr 25, 2016

LGTM

@dhermes dhermes merged commit 8491da2 into googleapis:master Apr 25, 2016
@rimey rimey deleted the import-star branch April 25, 2016 17:35
@tseaver tseaver mentioned this pull request May 16, 2016
Merged
parthea added a commit that referenced this pull request Nov 24, 2025
@renovate-bot @parthea
chore(deps): update all dependencies (#1747)
729ae56 
Co-authored-by: Anthonios Partheniou <[email protected]>
parthea pushed a commit that referenced this pull request Nov 26, 2025
@ohmayr @gcf-owl-bot
feat: add request response logging to auth (#1678)
7fa3e06 
* feat: add functionality to hash data (#1677)

* feat: add functionality to hash data

* change sensitive fields to private

* update to sha512

* update docstring

* 🦉 Updates from OwlBot post-processor

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

* chore: add request-response log helpers (#1685)

* chore: add request-response log helpers

* fix presubmit

* 🦉 Updates from OwlBot post-processor

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

* feat: opt-in logging support for request / response (#1686)

* feat: opt-in logging support for request/response

* add pragma no cover

* add test coverage for request/response

* add code coverage

* 🦉 Updates from OwlBot post-processor

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

* chore: remove logging for async requests (#1698)

* chore: remove logging for async requests

* change Dict to Mapping

* fix mypy and lint issues

* address PR feedback

* link issue

* feat: parse request/response for logging (#1696)

* feat: parse request/response for logging

* add test case for list

* address PR comments

* address PR feedback

* fix typo

* add test coverage

* add code coverage

* feat: hash sensitive info in logs (#1700)

* feat: hash sensitive info in logs

* make helper private

* add code coverage

* address PR feedback

* fix mypy type issue

* 🦉 Updates from OwlBot post-processor

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

* feat: add support for async response log (#1733)

* feat: add support for async response log

* fix whitespace

* add await

* add code coverage

* fix lint

* fix lint issues

* address PR feedback

* address PR feedback

* link issue

* feat: add request response logs for sync api calls (#1747)

* fix: remove dependency on api-core for logging (#1748)

* fix: remove dep on api-core for logging

* disable propagation to the root logger

* update async helpers tests

* fix lint issue

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

api: monitoring Issues related to the Cloud Monitoring API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rimey @tseaver @dhermes @jgeewax @googlebot