Large number of style-related changes to pass pylint by dhermes · Pull Request #238 · googleapis/google-cloud-python · GitHub

This is not to be merged, but should be reviewed and scrutinized. /cc @silvolu @tseaver

This is upstream of #235 as that was motivated by this larger effort towards fixing #179 and #178.

As for review, we should particularly discuss the things I pointed out.

Some key things to notice:

The tools (pep8 / pylint) disagree about continutation lines.
This is because PEP8 is ambiguous about "multi-line constructs".
One of the examples for bracket indentation in the PEP is
```
my_list = [
    1, 2, 3,
    4, 5, 6,
    ]
```
but the PEP doesn't explicity say whether or not
```
my_list = [1, 2, 3,
           4, 5, 6,
           ]
```
would be allowed. As it turns out, pep8 (the tool) considers
the latter to be a continuation while pylint does not.
I changed all indentation where this was a problem, but was
intentionally inconsistent. This way we can see "all options"
and discuss whichever is preferred.
I made a _MetadataMixin helper class for methods shared between
storage.Bucket and storage.Key. I did not refactor the affected
tests but they do still give 100% coverage.
To silence docstrings warnings I added many bogus docstrings
without actually documenting the given objects.
I factored out part of
```
storage.connection.Connection.generate_signed_url
```
into a _get_expiration_seconds() method and added tests.
This method is untested (has # pragma NO COVER) and is still very long.
I refactored because pylint complained about too many locals.
I made sure tox -e lint and tox -e cover passed.
I increased the maximum number of arguments from 5 to 10 and
the maximum public methods from 20 to 30. Offenders were:
- storage.connection.Connection.make_request (6 arguments)
- storage.connection.Connection.api_request (9 arguments)
- storage.connection.Connection.generate_signed_url (6 arguments)
- storage.bucket.Bucket (27 public methods) and
- storage.key.Key (23 public methods)
I think refactoring these should be considered (though is not
mandatory). I certainly don't know a good solution for the class
public methods.

@tseaver I also forgot to mention that I needed a mock for datetime.datetime.utcnow in test__get_expiration_seconds (in storage/test_connection.py) .

I just did the first thing that came to mind, but it is a bit hacky. So

Do we have a standard mocking approach / library that we want to use.
Do you have any insights specific to mocking datetime. (It was kind of a pain.)

Hmm, tests are failing for me (maybe a timezone-related issue?):

py27 create: /home/tseaver/projects/agendaless/Google/src/gcloud-python/.tox/py27
py27 installdeps: nose, unittest2
py27 inst: /home/tseaver/projects/agendaless/Google/src/gcloud-python/.tox/dist/gcloud-0.02.2.zip
py27 runtests: PYTHONHASHSEED='3555183268'
py27 runtests: commands[0] | nosetests
.....................................................................................................................................................................................................................................................................................................FF..............................................................................................................
======================================================================
FAIL: test__get_expiration_seconds (gcloud.storage.test_connection.TestConnection)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/tseaver/projects/agendaless/Google/src/gcloud-python/gcloud/storage/test_connection.py", line 526, in test__get_expiration_seconds
    CALL_FUT(expiration_no_tz))
AssertionError: 1092902400 != 1092891600

======================================================================
FAIL: test__get_expiration_seconds_w_timedelta (gcloud.storage.test_connection.TestConnection)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/tseaver/projects/agendaless/Google/src/gcloud-python/gcloud/storage/test_connection.py", line 583, in test__get_expiration_seconds_w_timedelta
    CALL_FUT(expiration_as_delta))
AssertionError: 1092902400 != 1092891600

@tseaver I think I don't understand the expected behavior well enough. You'll notice the difference is 3 hours, the difference between our local time zones. This must mean the relevant code

expiration.replace(tzinfo=pytz.utc)

is actually assuming the datetime.datetime object has the local timezone?

WRT mocking datetime, I've done the following (e.g., at module scope of the MUT):

_UTCNOW = None # unit tests can replace
def _utcnow(): #pragma NO COVERAGE
    if _UTCNOW is not None:
        return _UTCNOW
    return datetime.datetime.utcnow()

Later in the module, replace 'datetime.datetime.utcnow()' with '_utcnow()'.

and then in a test:

def test_foo(self):
    import datetime
    from gcloud import credentials as MUT

    utcnow = datetime.datetime.utcnow()
    with _Monkey(MUT, _UTCNOW=utcnow):
        result = self._callFUT(1000)
    self.assertEqual(result, utcnow + datetime.timedelta(0, 1000)

Rebased to master after #235 was merged.

@tseaver Since datetime.datetime is not a pure Python class, the attributes can't be patched.

>>> import datetime
>>> datetime.datetime.utcnow = None
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: can't set attributes of built-in/extension type 'datetime.datetime'

BTW @tseaver this PR can't be merged until we resolve the discussion, so the rebase is not so important.

Here are the datastore docstrings for you to handle:
https://gist.github.com/dhermes/ab246529ad1e2ae0b9bf

These are the rest, I will tackle:
https://gist.github.com/dhermes/a52007c58557419a310d

Found the issue and it wasn't with datetime. I'm happy we tested this because time.mktime computes a local time, which the method goes out of it's way to avoid. See StackOverflow for details.

@dhermes I wasn't suggesting patching the 'datetime.datetime' class: I was patching the 'gcloud.credentials' module.

Coverage remained the same when pulling 111ce66 on dhermes:large-style-changes into 61232de on GoogleCloudPlatform:master.

@tseaver I see now, sorry I read too quickly.

NOTE: I just rebased on top of #241

Coverage remained the same when pulling af1d5d7 on dhermes:large-style-changes into 61232de on GoogleCloudPlatform:master.

Coverage remained the same when pulling 936ff32 on dhermes:large-style-changes into 61232de on GoogleCloudPlatform:master.

Coverage remained the same when pulling 9fa9d90 on dhermes:large-style-changes into 61232de on GoogleCloudPlatform:master.

Coverage remained the same when pulling 76a8d05 on dhermes:large-style-changes into 11c8765 on GoogleCloudPlatform:master.

Coverage remained the same when pulling f2bef6e on dhermes:large-style-changes into fba037f on GoogleCloudPlatform:master.

Source-Link: https://togithub.com/googleapis/synthtool/commit/26c7505b2f76981ec1707b851e1595c8c06e90fc Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f946c75373c2b0040e8e318c5e85d0cf46bc6e61d0a01f3ef94d8de974ac6790

* chore(python): use cov_level in unittest gh action Source-Link: googleapis/synthtool@e5aaa84 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:d22cd2ddce65fdac6986f115563faf2fc81482b09dfbea83ac2808c92ecfdff0 * set coverage to 100% * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.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>

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

🤖 I have created a release *beep* *boop* --- ## [1.7.0](googleapis/python-bigquery-connection@v1.6.0...v1.7.0) (2022-08-02) ### Features * Add service_account_id output field to CloudSQL properties ([#237](googleapis/python-bigquery-connection#237)) ([adc73a6](googleapis/python-bigquery-connection@adc73a6)) ### Documentation * deprecate the AwsCrossAccountRole property ([#240](googleapis/python-bigquery-connection#240)) ([ad17197](googleapis/python-bigquery-connection@ad17197)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

Source-Link: googleapis/synthtool@453a5d9 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:81ed5ecdfc7cac5b699ba4537376f3563f6f04122c4ec9e735d3b3dc1d43dd32

PiperOrigin-RevId: 389649385 Source-Link: googleapis/googleapis@2c1c2ac Source-Link: googleapis/googleapis-gen@a10d84a

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: - [x] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/python-dlp/issues/new/choose) 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) Fixes #238 🦕

Source-Link: googleapis/synthtool@56da63e Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:993a058718e84a82fda04c3177e58f0a43281a996c7c395e0a56ccc4d6d210d7

Source-Link: googleapis/synthtool@4760d8d Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f0e4b51deef56bed74d3e2359c583fc104a8d6367da3984fc5c66938db738828 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

* chore: delete owlbot.py * 🦉 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>

* fix: Add async context manager return types chore: Mock return_value should not populate oneof message fields chore: Support snippet generation for services that only support REST transport chore: Update gapic-generator-python to v1.11.0 PiperOrigin-RevId: 545430278 Source-Link: googleapis/googleapis@601b532 Source-Link: googleapis/googleapis-gen@b3f18d0 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYjNmMThkMGY2NTYwYTg1NTAyMmZkMDU4ODY1ZTc2MjA0NzlkN2FmOSJ9 * 🦉 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>

[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [google-cloud-asset](https://togithub.com/googleapis/python-asset) | `==3.2.0` -> `==3.2.1` | [![age](https://badges.renovateapi.com/packages/pypi/google-cloud-asset/3.2.1/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/google-cloud-asset/3.2.1/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/google-cloud-asset/3.2.1/compatibility-slim/3.2.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/google-cloud-asset/3.2.1/confidence-slim/3.2.0)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>googleapis/python-asset</summary> ### [`v3.2.1`](https://togithub.com/googleapis/python-asset/blob/master/CHANGELOG.md#321-httpswwwgithubcomgoogleapispython-assetcomparev320v321-2021-07-21) [Compare Source](https://togithub.com/googleapis/python-asset/compare/v3.2.0...v3.2.1) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box. --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/python-asset).

….0 (#238) Co-authored-by: Anthonios Partheniou <partheniou@google.com>

Source-Link: googleapis/synthtool@453a5d9 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:81ed5ecdfc7cac5b699ba4537376f3563f6f04122c4ec9e735d3b3dc1d43dd32

Source-Link: https://togithub.com/googleapis/synthtool/commit/30bd01b4ab78bf1b2a425816e15b3e7e090993dd Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:9bc5fa3b62b091f60614c08a7fb4fd1d3e1678e326f34dd66ce1eefb5dc3267b

* feat: Add VULNERABILITY_ASSESSMENT Note type to grafeas v1 API, adds Vex_Assessment derived from the Note to resources' occurrences, VEX notes now be written to add CVE assessments PiperOrigin-RevId: 515727862 Source-Link: googleapis/googleapis@a4e6205 Source-Link: googleapis/googleapis-gen@3bc42dc Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiM2JjNDJkY2EyOTAwODE1YzE2NWNmN2QzNDE5ZmY3MGRmMDVkZmI5MCJ9 * 🦉 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>

Source-Link: googleapis/synthtool@050953d Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:65e656411895bff71cffcae97246966460160028f253c2e45b7a25d805a5b142 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

…cp/templates/python_library/.kokoro (#238) Source-Link: googleapis/synthtool@b4fe62e Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:3bf87e47c2173d7eed42714589dc4da2c07c3268610f1e47f8e1a30decbfc7f1 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

Source-Link: googleapis/synthtool@d52e638 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:4f9b3b106ad0beafc2c8a415e3f62c1a0cc23cabea115dbe841b848f581cfe99 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Victor Chudnovsky <vchudnov@google.com>

This file was redundant with/hidden by `google/cloud/bigtable/__init__.py` and seems to have been left in as an oversight.

Implementation for #237

🤖 I have created a release \*beep\* \*boop\* --- ## [2.2.0](https://www.github.com/googleapis/python-datastore/compare/v2.1.6...v2.2.0) (2021-10-08) ### Features * add support for Python 3.10 ([#233](https://www.github.com/googleapis/python-datastore/issues/233)) ([f524c40](https://www.github.com/googleapis/python-datastore/commit/f524c40e8251c2b716ea87cd512404f0d6f1b019)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

beriberikix added the in progress label Oct 13, 2014

dhermes added testing hygiene labels Oct 13, 2014

tseaver mentioned this pull request Oct 13, 2014

Add pyflakes to 'tox -e lint'. #229

Closed

tseaver reviewed Oct 13, 2014
View reviewed changes

Comment thread gcloud/storage/bucket.py Outdated

This comment was marked as spam.

Sign in to view

dhermes force-pushed the large-style-changes branch from f2e59d0 to 7da4349 Compare October 13, 2014 20:46

This was referenced Oct 13, 2014

Making all modules in gcloud package pylint compliant. #210

Closed

Adding base pylint rcfile and plugin for unittests. #221

Closed

tseaver mentioned this pull request Oct 13, 2014

Add gcloud.* and gcloud.datastore.* docstrings to pacify pylint. #240

Merged

dhermes force-pushed the large-style-changes branch from fa5da1e to af1d5d7 Compare October 13, 2014 23:21

dhermes force-pushed the large-style-changes branch from 9fa9d90 to 76a8d05 Compare October 14, 2014 02:34

dhermes force-pushed the large-style-changes branch from 76a8d05 to f2bef6e Compare October 14, 2014 02:43

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

chore(deps): update dependency grpcio to v1.49.0 (#238 )

1caa752

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

chore(deps): update dependency pytest to v7.1.0 (#238 )

fcbad7d

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

build: remove broken v1beta2 links (#238 )

9ce8f1f

PiperOrigin-RevId: 389649385 Source-Link: googleapis/googleapis@2c1c2ac Source-Link: googleapis/googleapis-gen@a10d84a

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

chore(deps): update dependency pytest to v7.1.0 (#238 )

63ce4b0

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

chore(deps): update dependency pytest to v7.1.0 (#238 )

55433d6

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

chore(deps): update dependency google-cloud-containeranalysis to v2.7…

21f3101

….0 (#238) Co-authored-by: Anthonios Partheniou <partheniou@google.com>

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

docs(samples): updated var name to avoid shadowing built-in (#238 )

c96016e

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

chore(deps): update dependency google-cloud-vision to v2.4.3 (#238 )

58bb184

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

chore(main): release 2.8.0 (#238 )

94ae7a6

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

parthea pushed a commit that referenced this pull request Jan 10, 2025

chore(deps): update all dependencies (#238 )

62a149d

parthea pushed a commit that referenced this pull request Nov 22, 2025

chore: remove google/cloud/bigtable.py (#238 )

f550607

This file was redundant with/hidden by `google/cloud/bigtable/__init__.py` and seems to have been left in as an oversight.

parthea pushed a commit that referenced this pull request Nov 24, 2025

Raw page (#238 )

d324ff2

Implementation for #237

Large number of style-related changes to pass pylint by dhermes · Pull Request #238 · googleapis/google-cloud-python · GitHub

Uh oh!

Large number of style-related changes to pass pylint - #238

Large number of style-related changes to pass pylint#238dhermes merged 2 commits intogoogleapis:mastergoogleapis/google-cloud-python:masterfrom dhermes:large-style-changesdhermes/google-cloud-python:large-style-changesCopy head branch name to clipboard

Conversation

dhermes commented Oct 13, 2014

Uh oh!

dhermes commented Oct 13, 2014

Uh oh!

tseaver commented Oct 13, 2014

Uh oh!

dhermes commented Oct 13, 2014

Uh oh!

This comment was marked as spam.

Uh oh!

tseaver commented Oct 13, 2014

Uh oh!

dhermes commented Oct 13, 2014

Uh oh!

dhermes commented Oct 13, 2014

Uh oh!

dhermes commented Oct 13, 2014

Uh oh!

dhermes commented Oct 13, 2014

Uh oh!

tseaver commented Oct 13, 2014

Uh oh!

coveralls commented Oct 13, 2014

Uh oh!

dhermes commented Oct 13, 2014

Uh oh!

dhermes commented Oct 13, 2014

Uh oh!

coveralls commented Oct 13, 2014

Uh oh!

coveralls commented Oct 13, 2014

Uh oh!

coveralls commented Oct 14, 2014

Uh oh!

coveralls commented Oct 14, 2014

Uh oh!

coveralls commented Oct 14, 2014

Uh oh!

coveralls commented Oct 14, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Large number of style-related changes to pass pylint#238
dhermes merged 2 commits into
googleapis:masterfrom
dhermes:large-style-changes