Skip to content

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