Skip to content

Conversation

michaelbrewer
Copy link
Contributor

@michaelbrewer michaelbrewer commented Jul 2, 2021

Issue #, if available:

#503

Description of changes:

Set the formatted output to include milliseconds

fixes #503

Checklist

Breaking change checklist

Potentially a breaking change if expecting only seconds.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 2, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jul 2, 2021

Codecov Report

Merging #504 (b08802e) into develop (cc128ad) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #504   +/-   ##
========================================
  Coverage    99.90%   99.90%           
========================================
  Files          105      105           
  Lines         4234     4234           
  Branches       206      206           
========================================
  Hits          4230     4230           
  Misses           1        1           
  Partials         3        3           
Impacted Files Coverage Δ
...ilities/data_classes/appsync/scalar_types_utils.py 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 cc128ad...b08802e. Read the comment docs.

@michaelbrewer
Copy link
Contributor Author

@heitorlessa I think that %f might cause issues with AppSync dates as they are expecting milliseconds.

@michaelbrewer
Copy link
Contributor Author

@heitorlessa seems to work as microseconds, so should i still trim off the extra 3 positions? or leave it

@heitorlessa heitorlessa added the bug Something isn't working label Jul 5, 2021
@heitorlessa
Copy link
Contributor

thanks for the fix!! Let's trim off to stay as milliseconds ([:-3]) to stay compliant whether validation catches server-side - Depending on how much data one pumps in, this should also save on costs

Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

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

Trim last three to stick as ms [:-3]

@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 5, 2021
Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, Mike!

Note to self: break down test_data_classes as it's 1.3K LOC atm

@heitorlessa heitorlessa merged commit 2473480 into aws-powertools:develop Jul 6, 2021
@michaelbrewer
Copy link
Contributor Author

Thanks for the changes, Mike!

Note to self: break down test_data_classes as it's 1.3K LOC atm

It would be great if pytest can run parts in parallel :). Then i would definitely help split up things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: aws scalar type for AWSDateTime should include milliseconds
3 participants