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

vdk-core: implement config option for logging execution result #2831

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

DeltaMichael
Copy link
Contributor

@DeltaMichael DeltaMichael commented Oct 24, 2023

Why?

User reasearch indicates that the execution result is too verbose for local runs. Users don't expect a lot of output for successful jobs and expect error output for failing jobs.

What?

Introduce the LOG_EXECUTION_RESULT config option that enables/disables displaying the end result.

Config Error Output

https://pastebin.com/raw/MvJX0u9m

User Error Output

https://pastebin.com/raw/0MxZp6Zg

Success Output

https://pastebin.com/raw/GTNBiEFG

How was this tested?

Ran locally with successful and failing jobs

What kind of change is this?

Feature/non-breaking

@antoniivanov
Copy link
Collaborator

antoniivanov commented Oct 24, 2023

Please add tests.

I think the easiest way to test them would be with functional test:

    runner = CliEntryBasedTestRunner()

    result: Result = runner.invoke(["run", util.job_path("fail-job-indirect-library")])

the result object contain stderr and std out which you can inspect for verification.

@DeltaMichael DeltaMichael force-pushed the person/mdilyan/log-end-result branch 4 times, most recently from b0c3fcb to d04b8fe Compare October 25, 2023 22:49
@DeltaMichael DeltaMichael marked this pull request as draft October 27, 2023 11:53
@DeltaMichael DeltaMichael force-pushed the person/mdilyan/log-end-result branch 4 times, most recently from 4fd4874 to 669ce77 Compare October 30, 2023 11:49
DeltaMichael pushed a commit that referenced this pull request Oct 30, 2023
Why?

User reasearch indicates that the execution result is too verbose for local runs. Users don't expect a lot of output for successful jobs and expect error output for failing jobs.

What?

Introduce the LOG_EXECUTION_RESULT config option that enables/disables displaying the end result.
Enabling and disabling the end result will be implemented in a separate PR.
#2831

How was this tested?

CI

What kind of change is this?

Feature/non-breaking

Signed-off-by: Dilyan Marinov <mdilyan@vmware.com>
@DeltaMichael DeltaMichael changed the base branch from main to person/mdilyan/log-end-result-conf October 30, 2023 13:01
@DeltaMichael DeltaMichael changed the title vdk-core: add config option for logging execution result vdk-core: implement config option for logging execution result Oct 30, 2023
Base automatically changed from person/mdilyan/log-end-result-conf to main October 30, 2023 14:09
DeltaMichael added a commit that referenced this pull request Oct 30, 2023
## Why?

User reasearch indicates that the execution result is too verbose for
local runs. Users don't expect a lot of output for successful jobs and
expect error output for failing jobs.

## What?

Introduce the LOG_EXECUTION_RESULT config option that enables/disables
displaying the end result. Enabling and disabling the end result will be
implemented in a separate PR.

#2831

## How was this tested?

CI

## What kind of change is this?

Feature/non-breaking

Signed-off-by: Dilyan Marinov <mdilyan@vmware.com>
@DeltaMichael DeltaMichael marked this pull request as ready for review November 13, 2023 07:58
@DeltaMichael DeltaMichael force-pushed the person/mdilyan/log-end-result branch 2 times, most recently from 0a1793c to 1cd13d7 Compare November 14, 2023 08:57
@DeltaMichael DeltaMichael force-pushed the person/mdilyan/log-end-result branch 2 times, most recently from 76c87dd to a1f4872 Compare November 14, 2023 10:14
Why?

User reasearch indicates that the execution result is too verbose
for local runs. Users don't expect a lot of output for successful
jobs and expect error output for failing jobs.

What?

Introduce the LOG_EXECUTION_RESULT config option that enables/disables displaying the
end result.

How was this tested?

Ran locally with successful and failing jobs
Functional tests

What kind of change is this?

Feature/non-breaking

Signed-off-by: Dilyan Marinov <mdilyan@vmware.com>
@DeltaMichael DeltaMichael enabled auto-merge (squash) November 15, 2023 08:25
@DeltaMichael DeltaMichael merged commit c5c6a2d into main Nov 15, 2023
7 checks passed
@DeltaMichael DeltaMichael deleted the person/mdilyan/log-end-result branch November 15, 2023 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Put logging of the data job summary behind a config option in vdk-core
5 participants