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

Sending the executions in UTC-0 (rel. issue 20) #25

Merged
merged 2 commits into from
Apr 2, 2023
Merged

Sending the executions in UTC-0 (rel. issue 20) #25

merged 2 commits into from
Apr 2, 2023

Conversation

pebeto
Copy link
Member

@pebeto pebeto commented Mar 27, 2023

  • MLFlow executions need to be tracked in UTC-0
  • An extra effort regarding issues tests in an extra directory.

Note:
MLFlow dashboard shows the local UTC configuration, meaning that we are not going to be able to see changes (unless we change to another machine with another timezone).

test/runtests.jl Outdated Show resolved Hide resolved
)
]
)
@test isa(mlf_run, MLFlowRun)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was wondering if we could ensure that the timestamp is actually in UTC, but it seems that DateTime doesn't keep that information:

julia> dt = now()
2023-03-28T12:26:28.664

julia> dump(dt)
DateTime
  instant: Dates.UTInstant{Millisecond}
    periods: Millisecond
      value: Int64 63815689588664

julia> dt = now(UTC)
2023-03-28T11:26:38.460

julia> dump(dt)
DateTime
  instant: Dates.UTInstant{Millisecond}
    periods: Millisecond
      value: Int64 63815685998460

julia> 

can you think of a way to test whether the timestamp is acutally in UTC? if not - no problem, let's just sort out the change in the regular expression below and merge.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've looked for a way but I found nothing about checking that. It seems kinda impossible unless we do an outside call to ensure if Julia's UTC is actually UTC.
I'm uploading the regex change now.

@codecov-commenter
Copy link

Codecov Report

Merging #25 (a2e89f3) into main (d1afb2e) will increase coverage by 0.10%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main      #25      +/-   ##
==========================================
+ Coverage   70.25%   70.35%   +0.10%     
==========================================
  Files           4        5       +1     
  Lines         279      280       +1     
==========================================
+ Hits          196      197       +1     
  Misses         83       83              
Impacted Files Coverage Δ
src/runs.jl 53.54% <100.00%> (ø)

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@deyandyankov deyandyankov merged commit 70e97c8 into JuliaAI:main Apr 2, 2023
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.

3 participants