Skip to content
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: minimum datetime should be in iso format #122

Merged
merged 2 commits into from
Aug 28, 2024

Conversation

renaudjester
Copy link
Collaborator

No description provided.

@renaudjester renaudjester requested a review from uriii3 August 27, 2024 14:57
@renaudjester renaudjester force-pushed the fix-format-min-datetime-when-retention-date branch from bfef626 to 537654a Compare August 28, 2024 07:49
@uriii3
Copy link
Collaborator

uriii3 commented Aug 28, 2024

is this for CMT-100 ?

@@ -616,7 +616,7 @@ def _format_arco_data_metadata_producer_valid_start_date(
).timestamp()
* 1000
)
return arco_data_metadata_producer_valid_start_date.split(".")[0]
return arco_data_metadata_producer_valid_start_date
Copy link
Collaborator

Choose a reason for hiding this comment

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

okay, so basically when we are not returning it to a timestamp, it is okay to maintain (and they prefer it this way) the milliseconds and everything, no?
When are we sending it to timestamp and when not? In the code it appears isinstance(coordinates_info.get("min"), int) this but not sure what is the logic behind.

Copy link
Collaborator

@uriii3 uriii3 Aug 28, 2024

Choose a reason for hiding this comment

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

I mean I was checking the ones that were supposed to be timestamps and the ones that weren't and all looked the same... what is the difference behind them?

@renaudjester renaudjester merged commit 6c700ee into main Aug 28, 2024
2 checks passed
@renaudjester renaudjester deleted the fix-format-min-datetime-when-retention-date branch August 28, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants