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

[CT-649] [Feature] Prettify duration message at the end of execution #5253

Closed
1 task done
SOVALINUX opened this issue May 15, 2022 · 2 comments · Fixed by #5364
Closed
1 task done

[CT-649] [Feature] Prettify duration message at the end of execution #5253

SOVALINUX opened this issue May 15, 2022 · 2 comments · Fixed by #5364
Labels
enhancement New feature or request good_first_issue Straightforward + self-contained changes, good for new contributors! logging

Comments

@SOVALINUX
Copy link

Is this your first time opening an issue?

Describe the Feature

Hi colleagues,
Would like to suggest more human-readable logging for end of execution dbt commands
Now we log only time in seconds, but it's not very convenient to convert it in your head every time you need to understand how long it was
For example:

Finished running 15 tests, 1 hook in 410.20s.

I would suggest to write it in the following way:

Finished running 15 tests, 1 hook in 6 min 51s (410.20s).

Saving pure seconds for those cases who might be parsing the duration from logs using RegExp

Describe alternatives you've considered

No response

Who will this benefit?

Those who execute pipelines longer than few minutes

Are you interested in contributing this feature?

I think my team can do it

Anything else?

No response

@SOVALINUX SOVALINUX added enhancement New feature or request triage labels May 15, 2022
@github-actions github-actions bot changed the title [Feature] Prettify duration message at the end of execution [CT-649] [Feature] Prettify duration message at the end of execution May 15, 2022
@jtcohen6
Copy link
Contributor

@SOVALINUX Makes sense to me! We do something similar for bytes_processed on dbt-bigquery here. The underlying data is still recorded as an integer (number of bytes, number of seconds), but then we convert the value for inclusion in the human-friendly message.

The relevant message is in a event named HookFinished:

def print_results_line(self, results, execution_time):
nodes = [r.node for r in results] + self.ran_hooks
stat_line = get_counts(nodes)
execution = ""
if execution_time is not None:
execution = " in {execution_time:0.2f}s".format(execution_time=execution_time)
with TextOnly():
fire_event(EmptyLine())
fire_event(HookFinished(stat_line=stat_line, execution=execution))

@dataclass
class HookFinished(InfoLevel):
stat_line: str
execution: str
code: str = "E040"
def message(self) -> str:
return f"Finished running {self.stat_line}{self.execution}."

I'm going to mark this a good first issue. It sounds like it's a change your team may be interested in contributing?


Tech debt: I'll be honest, this event feels far from perfect:

  • HookFinished doesn't feel like the right name, versus RunResultsLine or similar. (I'm not sure if or how we could make this change in a backwards-compatible manner, though)
  • I'd much rather pass in a structured dictionary of node counts, and then render the human-friendly stat_line for the event's message
  • We should also be storing execution_time as a float, rather than execution as a string

Those are beyond the scope of the specific change you're proposing. For what you're after, I think it would be reasonable to:

  • Format execution, as a string, with hours/minutes/seconds (1 hr 6 min 51s)
  • Add execution_time (float, representing seconds) as a dataclass attribute that gets passed into HookFinished, and included in its structured (JSON) output, even if it isn't templated into the human-friendly message

@jtcohen6 jtcohen6 added good_first_issue Straightforward + self-contained changes, good for new contributors! Team:Execution logging and removed triage labels May 16, 2022
@SOVALINUX
Copy link
Author

Hi
Will take a look, thanks! It's not the first priority for my obviously, but sooner or later we can make it :)
Thanks for your inputs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good_first_issue Straightforward + self-contained changes, good for new contributors! logging
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants