qri-io / qri

you're invited to a data party!
https://qri.io
GNU General Public License v3.0
1.1k stars 66 forks source link

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

Closed ramfox closed 2 years ago

ramfox commented 2 years ago

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.