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

refactor(collection): frontend collection expects the "run end time" in its UI #1944

Closed
5 tasks done
ramfox opened this issue Oct 19, 2021 · 0 comments · Fixed by #1945
Closed
5 tasks done

refactor(collection): frontend collection expects the "run end time" in its UI #1944

ramfox opened this issue Oct 19, 2021 · 0 comments · Fixed by #1945
Assignees
Labels
refactor A code change that neither fixes a bug nor adds a feature
Projects

Comments

@ramfox
Copy link
Member

ramfox commented Oct 19, 2021

In logbook, we are currently using the CommitTime in the dsref.VersionInfo in place of a RunTime when we convert logs into version infos. This has been working fine from the view point of the Activity feed, because each version info is only there to describe a single run or a single commit, not the dataset as a whole. So based on whether or not a version info has a RunID (in this context), you can determine whether or not we are talking about a "run" activity or a "commit" activity.

However, this is muddied in the Collection view. Since the VersionInfo in the Collection view models the entire dataset as a whole, we have it structured that a user may have commit information from one run as well as run information about another run in the same version.

For example. If we run a workflow that successfully produces a commit, we catch that information in the Collection, which updates the RunID, RunDuration, RunCount, RunStatus, CommitCount, CommitTitle, CommitMessage, Path, and CommitTime.

Next, we run a workflow which runs successfully, but does not produce a commit (no changes). The collection will update the RunID, RunDuration, RunCount, and RunStatus, but leave the Commit specific fields alone.

So that means in two different parts of our codebase, the CommitTime field can maybe be the RunTime or the CommitTime, or is always the CommitTime.

Frontend, similarly, has been using CommitTime to correctly mean the RunTime in the ActivityLog, CommitTime in the DatasetHistory, and incorrectly mean RunTime in the Collection

I see two ways to solve:

  1. add a RunTime that tracks the start of a run. We can use the RunTime + RunDuration if we need to track the run end time
  2. rename the CommitTime field to Time, and consider it the RunTime when there is a RunID, or the CommitTime when there is no RunID. I think this is dumb & we should definitely do the former.
  • add RunTime to the VersionInfo definition
  • adjust runItemFromOp in logbook to pass the time into the RunTime field, rather than CommitTime
  • adjust the ETLogbookWriteRun event to pass the runState.StartTime in WriteTransformRun
  • adjust the Collection to store the RunTime from the ETLogbookWriteRun event
  • update tests ensuring the RunTime recorded correctly & is not lost when other events write to the Collection
@ramfox ramfox self-assigned this Oct 20, 2021
@ramfox ramfox added the refactor A code change that neither fixes a bug nor adds a feature label Oct 20, 2021
@ramfox ramfox added this to Backlog in Sprints via automation Oct 20, 2021
@ramfox ramfox moved this from Backlog to To do in Sprints Oct 20, 2021
@ramfox ramfox moved this from To do to In progress in Sprints Oct 20, 2021
Sprints automation moved this from In progress to Done Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor A code change that neither fixes a bug nor adds a feature
Projects
Development

Successfully merging a pull request may close this issue.

1 participant