-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
Fix variable nanosecond timestamp update_time from response #4819
Fix variable nanosecond timestamp update_time from response #4819
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Would a try except work better then or should we just wait?
|
I'm emailing the team to check. |
@tswast ping? |
I never got a response from the folks I've contacted before on the RuntimeConfig team. @mgannholm Can you comment on this? |
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. |
7432b55
to
7cd4bee
Compare
7cd4bee
to
2dc2357
Compare
2dc2357
to
8f38fee
Compare
@tswast PTAL at your convenience. |
|
||
VARIABLE_NAME = 'my-variable/abcd' | ||
VARIABLE_PATH = '%s/variables/%s' % (self.CONFIG_PATH, VARIABLE_NAME) | ||
RESOURCE = { | ||
'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.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
going to merge this and diff from this in the new PR. (I have it written already) |
@tswast Thanks for the hint.
Closes #4807