Skip to content

Conversation

@dhermes
Copy link
Contributor

@dhermes dhermes commented Aug 7, 2015

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 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
Copy link
Contributor

tseaver commented Aug 7, 2015

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
Copy link
Contributor Author

dhermes commented Aug 7, 2015

@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
Copy link
Contributor

tseaver commented Aug 7, 2015

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

@dhermes
Copy link
Contributor Author

dhermes commented Aug 7, 2015

Yeah for sure.

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

dhermes commented Aug 8, 2015

@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
Copy link
Contributor

tseaver commented Aug 8, 2015

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

@dhermes
Copy link
Contributor Author

dhermes commented Aug 8, 2015

What is that?

@tseaver
Copy link
Contributor

tseaver commented Aug 8, 2015

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
Copy link
Contributor Author

dhermes commented Aug 8, 2015

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

@tseaver
Copy link
Contributor

tseaver commented Aug 8, 2015

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
Copy link
Contributor Author

dhermes commented Aug 8, 2015

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

@tseaver
Copy link
Contributor

tseaver commented Aug 10, 2015

LGTM

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

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
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