FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Removing use of pytz module / implementing our own UTC. by dhermes · Pull Request #1044 · googleapis/google-cloud-python · GitHub

Removing use of pytz module / implementing our own UTC. - #1044

Merged
dhermes merged 1 commit into
googleapis:masterfrom
dhermes:remove-pytz-dependency
Aug 10, 2015
Merged

Removing use of pytz module / implementing our own UTC.#1044
dhermes merged 1 commit into
googleapis:masterfrom
dhermes:remove-pytz-dependency

Conversation

dhermes commented Aug 7, 2015

Copy link
Copy Markdown
Contributor

This was inspired by #1009.

Our methods for signing strings with credentials / crypto will likely be moved into oauth2client, so this is a pre-emptive move to avoid having pytz as a dependency for oauth2client.

As an added benefit, we avoid having pytz as a dependency, which helps for people wanting to use gcloud-python on App Engine.

/cc @nathanielmanistaatgoogle

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 7, 2015

This comment was marked as spam.

This comment was marked as spam.

tseaver commented Aug 7, 2015

Copy link
Copy Markdown
Contributor

No problem with this one overall. I wonder if our local UTC should be an alias of pytz.utc, IFF pytz is importable (to avoid having more than one global for the zone).

dhermes commented Aug 7, 2015

Copy link
Copy Markdown
Contributor Author

@tseaver RE: default to using pytz, default to ours. Good call!

Code reviews are so useful 😄 I totally overlooked that and it's totally the right way to go.

tseaver commented Aug 7, 2015

Copy link
Copy Markdown
Contributor

I figured that dropping pytz was really FBO users who never care about timezones at all: those who do will want it, anyway.

dhermes commented Aug 7, 2015

Copy link
Copy Markdown
Contributor Author

Yeah for sure.

dhermes force-pushed the remove-pytz-dependency branch from 790e798 to b8bce16 Compare August 8, 2015 00:35

dhermes commented Aug 8, 2015

Copy link
Copy Markdown
Contributor Author

@tseaver I re-pushed the initial commit. Sorry I finished it awhile ago but forgot to push.

Wanted to actually test the case that pytz was installed, but it was being too big a PITA to deal with sys.modules. WDYT?

tseaver commented Aug 8, 2015

Copy link
Copy Markdown
Contributor

We could add a tox stanza that didn't install it.

dhermes commented Aug 8, 2015

Copy link
Copy Markdown
Contributor Author

What is that?

tseaver commented Aug 8, 2015

Copy link
Copy Markdown
Contributor

Oops, I inverted it in my mind: we could add a stanza that tests it w/ pytz installed, e.g.:

[testenv:py27_w_extras]
deps =
    {[testenv]deps}
    pytz

dhermes commented Aug 8, 2015

Copy link
Copy Markdown
Contributor Author

Can we punt on it for this PR and fix address it after?

tseaver commented Aug 8, 2015

Copy link
Copy Markdown
Contributor

Sure.

The only thing I see which might be gating is we have no explicit test coverage for the _UTC class: do we need it? If not, go ahead and merge.

dhermes commented Aug 8, 2015

Copy link
Copy Markdown
Contributor Author

I added it to test__helpers.py, been in here for awhile.

tseaver commented Aug 10, 2015

Copy link
Copy Markdown
Contributor

LGTM

dhermes added a commit that referenced this pull request Aug 10, 2015
Removing use of pytz module / implementing our own UTC.
dhermes merged commit 99e04fd into googleapis:master Aug 10, 2015
dhermes deleted the remove-pytz-dependency branch August 10, 2015 14:40
Comment thread gcloud/test__helpers.py

This comment was marked as spam.

This comment was marked as spam.

parthea pushed a commit that referenced this pull request Nov 24, 2025
parthea pushed a commit that referenced this pull request Nov 24, 2025
parthea pushed a commit that referenced this pull request Nov 24, 2025
* Revert "test: add tests for autogen-snippets option (#1055)"

This reverts commit 185ecc7536db8309e6d2f03f9a66c36db18b1945.

* Revert "feat: generate code snippets by default (#1044)"

This reverts commit e46f443dbeffe16b63f97668801b06189769e972.
parthea pushed a commit that referenced this pull request Nov 24, 2025
Enable snippetgen for the default (non-Ads) templates.

This reverts commit 8bdb70931a9ecb1c89fda9608697b0762770bc12 (which was a revert of #1044 and #1055).

I've checked that the changes are OK (don't break generation for any APIs) by creating a [tag](https://github.com/googleapis/gapic-generator-python/commits/v0.62.0b1) and running the [presubmit](https://critique.corp.google.com/cl/424921742).
parthea pushed a commit that referenced this pull request Nov 26, 2025
* feat: add experimental GDCH support

* use ec key

* update comment

* Update google/oauth2/gdch_credentials.py

* fix

* add project, update payload
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: core cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL