+
Skip to content

Add Support For Millisecond Timestamps #1850

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

Merged
merged 2 commits into from
Apr 8, 2021
Merged

Conversation

lindluni
Copy link
Contributor

@lindluni lindluni commented Apr 6, 2021

The GitHub Audit Log API introduced timestamps with millisecond granularity. We add support for these timestamps by first parsing the timestamp into a Unix timestamp and then checking to see if the generated timestamp is in the future. If the timestamp is in the future we re-parse it as a Unix timestamp in Milliseconds.

Tests were added to ensure that when the timestamp is 1 millisecond off that the granularity of the timestamp is preserved by milliseconds correctly being parsed.

Closes #1849

Signed-off-by: Brett Logan lindluni@github.com

@google-cla google-cla bot added the cla: yes Indication that the PR author has signed a Google Contributor License Agreement. label Apr 6, 2021
@lindluni lindluni marked this pull request as draft April 6, 2021 02:59
@lindluni
Copy link
Contributor Author

lindluni commented Apr 6, 2021

Missed the enterprise_audit_log tests, fixing...

@lindluni lindluni marked this pull request as ready for review April 6, 2021 03:04
@codecov
Copy link

codecov bot commented Apr 6, 2021

Codecov Report

Merging #1850 (9dfcb8e) into master (bf34728) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1850   +/-   ##
=======================================
  Coverage   97.59%   97.59%           
=======================================
  Files         104      104           
  Lines        6615     6617    +2     
=======================================
+ Hits         6456     6458    +2     
  Misses         86       86           
  Partials       73       73           
Impacted Files Coverage Δ
github/timestamp.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf34728...9dfcb8e. Read the comment docs.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @lindluni !

The GitHub Audit Log API introduced timestamps
with millisecond granularity. We add support
for these timestamps by first parsing the timestamp
into a Unix timestamp and then checking to see if
the generated timestamp is in the future. If the
timestamp is in the future we re-parse it as a Unix
timestamp in Milliseconds.

Signed-off-by: Brett Logan <lindluni@github.com>
@gmlewis
Copy link
Collaborator

gmlewis commented Apr 6, 2021

lindluni force-pushed the lindluni:timestamp branch from b00be51 to b3adf37 10 minutes ago

Just a friendly reminder - please don't force-push to PRs in this repo in the future, as it makes it more difficult for reviewers to see what changed since the last review. Thanks. 😄

Signed-off-by: Brett Logan <lindluni@github.com>
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @lindluni !
LGTM.

Awaiting second LGTM before merging.

@gmlewis gmlewis requested a review from wesleimp April 6, 2021 14:32
Copy link
Collaborator

@wesleimp wesleimp left a comment

Choose a reason for hiding this comment

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

💙

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 8, 2021

Thank you, @wesleimp !
Merging.

@gmlewis gmlewis merged commit d711542 into google:master Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indication that the PR author has signed a Google Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Audit Log API Returns Non-Standard Timestamp
3 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载