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

Fix variable nanosecond timestamp update_time from response by chemelnucfin · Pull Request #4819 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

Fix variable nanosecond timestamp update_time from response - #4819

Merged
chemelnucfin merged 2 commits into
googleapis:masterfrom
chemelnucfin:issue4807-runtime_config_nanoseconds
Apr 4, 2018
ConversationCommits (2)ChecksFiles changed
Merged

Fix variable nanosecond timestamp update_time from response#4819
chemelnucfin merged 2 commits into
googleapis:masterfrom
chemelnucfin:issue4807-runtime_config_nanoseconds

Conversation

chemelnucfin commented Jan 31, 2018

Copy link
Copy Markdown
Contributor

@tswast Thanks for the hint.

Closes #4807

chemelnucfin added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. api: runtimeconfig Issues related to the Cloud Runtime Config API API. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Jan 31, 2018
chemelnucfin self-assigned this Jan 31, 2018
chemelnucfin requested a review from tswast as a code owner January 31, 2018 23:35
googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 31, 2018
tswast approved these changes Jan 31, 2018
View reviewed changes

tswast left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM, though we might need a fallback in case microseconds appear again.

We should check with the product team to see what they are committing to.

chemelnucfin commented Jan 31, 2018

Copy link
Copy Markdown
Contributor Author

Would a try except work better then or should we just wait?

try:
  return ms
except ValueError:
  return ns

tswast commented Feb 1, 2018

Copy link
Copy Markdown
Contributor

I'm emailing the team to check.

chemelnucfin changed the title Runtime Config: Fix variable nanosecond timestamp update_time from response Fix variable nanosecond timestamp update_time from response Feb 20, 2018
tseaver removed type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Feb 20, 2018

tseaver commented Mar 5, 2018

Copy link
Copy Markdown
Contributor

@tswast ping?

tswast commented Mar 5, 2018

Copy link
Copy Markdown
Contributor

I never got a response from the folks I've contacted before on the RuntimeConfig team.

@mgannholm Can you comment on this?

tswast commented Mar 6, 2018

Copy link
Copy Markdown
Contributor

Okay, I got confirmation from Victor on the RuntimeConfig team that "it might change from platform to platform". We need to be able to handle both.

Yes, try-except would be a good way to do it. Alternatively, I believe we have places (in BigQuery, maybe?) where we try to parse the string a little bit to see how many digits we have before trying to parse.

chemelnucfin commented Mar 6, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I'll wait until the datetime PRs #4979, #4980, #4981 are merged before finishing this up.

chemelnucfin force-pushed the issue4807-runtime_config_nanoseconds branch 2 times, most recently from 7432b55 to 7cd4bee Compare March 25, 2018 18:01
chemelnucfin force-pushed the issue4807-runtime_config_nanoseconds branch from 7cd4bee to 2dc2357 Compare April 4, 2018 22:04
chemelnucfin force-pushed the issue4807-runtime_config_nanoseconds branch from 2dc2357 to 8f38fee Compare April 4, 2018 22:19

chemelnucfin commented Apr 4, 2018

Copy link
Copy Markdown
Contributor Author

@tswast PTAL at your convenience.

tswast reviewed Apr 4, 2018
View reviewed changes
Comment thread runtimeconfig/tests/unit/test_config.py
'name': VARIABLE_PATH,
'value': 'bXktdmFyaWFibGUtdmFsdWU=', # base64 my-variable-value
'updateTime': '2016-04-14T21:21:54.5000Z',
'updateTime': '2016-04-14T21:21:54.123456789Z',

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

chemelnucfin commented Apr 4, 2018

Copy link
Copy Markdown
Contributor Author

going to merge this and diff from this in the new PR. (I have it written already)

chemelnucfin merged commit 9a6acfa into googleapis:master Apr 4, 2018
parthea pushed a commit that referenced this pull request Nov 24, 2025
Fix variable nanosecond timestamp update_time from response (#4819)
2b57061
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

tswast tswast approved these changes

Assignees

chemelnucfin

Labels

api: runtimeconfig Issues related to the Cloud Runtime Config API 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.

4 participants

Back | FazBrowse Home | New Git URL