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

Add command to diff the inputs of 2 builds #120

Closed
fho opened this issue Apr 16, 2020 · 5 comments · Fixed by #204
Closed

Add command to diff the inputs of 2 builds #120

fho opened this issue Apr 16, 2020 · 5 comments · Fixed by #204

Comments

@fho
Copy link
Collaborator

fho commented Apr 16, 2020

Add a command that shows the list of inputs that are different between two builds:

The usage could be like:
baur diff inputs <APP-NAME>.<TASK-NAME>|<TASK-RUN-ID> <APP-NAME>.<TASK-NAME>|<TASK-RUN-ID>

Parameters to support:

  • --quiet, do not show anything, exit with 0 if the inputs are the same, otherwise with 1
  • --csv if it fits with the format that we choose to display the diff

Optional: support to specify a syntax like gits HEAD^^ to show the last build or last-X build of an application.
Idea:

  • <APP-NAME>.<TASK-NAME> -> uses the inputs of the current files of the task in the filesystem,
  • <APP-NAME>.<TASK-NAME>^ -> diffs with the last run of the task
  • <APP-NAME>.<TASK-NAME>^^ diffs with the tasks before the last build,

Source: #117 (comment)_

@fho fho added this to the Version 1.0 milestone Apr 16, 2020
@fho fho removed this from the Version 1.0 milestone Sep 2, 2020
@dil-spayne
Copy link
Contributor

dil-spayne commented Oct 1, 2020

@fho I've started putting some test case scenarios together for this work and I would appreciate your input.
Can you please review the following and help me answer the questions in the Comments column.
Also, do you have a preferred format for the Output?

Data Setup

Application.Task RunID Inputs
app_one.build n/a, current files file_one - sha384:321
file_two - sha384:456
file_four - sha384:987
app_one.build 1 file_one - sha384:123
app_one.build 2 file_one - sha384:321
file_two - sha384:456
file_three - sha384:789
app_two.compile n/a, current files file_one - sha384:123
file_two - sha384:456
app_two.compile 3 file_one - sha384:123
file_two - sha384:456

Test Cases

Argument One Argument Two Output Exit code
app_one.build   Error: accepts 2 args, received x 1
app_one.build^   Error: accepts 2 args, received x 1
1 Error: accepts 2 args, received x 1
app_one app_two.compile  Error: app_one does not specify a task 1
app_one.build^^^ 1 Error: app_one.build^^^ does not exist, (only found 2 recorded runs in the database) 1
app_one.build app_one.build Error: app_one.build and app_one.build refer to the same task-run 1
app_one.build^^ 1 Error: app_one.build^^ and 1 refer to the same task-run 1
app_one.build 99 Error: task-run 99 does not exist 1
unknown.thing 1 Error: task does not exist 1
app_one.build app_one.build^ 
StatePathDigest1Digest2
+file_threesha384:789
-file_foursha384:987
2
app_one.build app_one.build^^ 
StatePathDigest1Digest2
Dfile_onesha384:321sha384:123
-file_twosha384:456
-file_foursha384:987
2
app_one.build 1
StatePathDigest1Digest2
Dfile_onesha384:321sha384:123
-file_twosha384:456
-file_foursha384:987
2
1 2
StatePathDigest1Digest2
Dfile_onesha384:123sha384:321
+file_twosha384:456
+file_threesha384:789
2
app_one.build app_two.compile
StatePathDigest1Digest2
Dfile_onesha384:321sha384:123
-file_foursha384:987
2
app_one.build^ app_two.compile
StatePathDigest1Digest2
Dfile_onesha384:321sha384:123
-file_threesha384:789
2
app_two.compile 3
StatePathDigest1Digest2
0

@fho
Copy link
Collaborator Author

fho commented Oct 5, 2020

@StephenDiligent

My usage description seemed to have been unclear.
With the introduction of tasks, it also changed a bit (I'll also update the description.).

The syntax<APP-NAME>.<TASK-NAME>|<TASK-RUN-ID> should mean either to specify the task-name of an app or a task-run-id.

For example:

baur diff inputs calc.build 1 # compare the current inputs of the build task of the calc application with the ones of task-run 1
baur diff inputs 1 2 # compare the inputs of run 1 and 2
baur diff inputs calc.build shop.test # compare the current inputs of the build task of the calc app with the current inputs of the test task of the shop app

@fho I've started putting some test case scenarios together for this work and I would appreciate your input.
Can you please review the following and help me answer the questions in the Comments column.

app_one app_two^^ ? 1 How do we handle “run doesn’t exist”?
app_one|3 app_two|3 ? 1 How do we handle “run doesn’t exist for either app”?
app_one app_three ? 1 How do we handle “app doesn’t exist”?

When an app or run-id that was specified as parameter does not exist, always print an error message and exit.
We don't not need to do anything in the case. The user provided parameter invalid values.

Also, do you have a preferred format for the Output?

I had a format like the following in mind:
$ baur diff inputs random.build 1:

State   Path            Digest1         Digest2
D       main.go         sha384:123      sha384:12345
+       test.go                         sha384:2950
-       hello.go        sha384:6891

The State column indicates the type of change:

  • D - different digest
  • + the inputs of the run/task specified by the second parameter has a file that is not part of the inputs of the run/task of specified by the first parameter
  • - analogous, the inputs is part of the task/run specified by the first parameter but not of the task/run specified by the second parameter

The Digest1 is the digest from the inputs of the first parameter, Digest2 of the second parameter.

We could color the lines according to the status on the console.
This format could also be printed in csv format (--csv) when using the table formatter.

What do you think?

Thanks for your help 🎉.

@fho fho linked a pull request Oct 5, 2020 that will close this issue
@dil-spayne
Copy link
Contributor

@StephenDiligent

My usage description seemed to have been unclear.
With the introduction of tasks, it also changed a bit (I'll also update the description.).

The syntax<APP-NAME>.<TASK-NAME>|<TASK-RUN-ID> should mean either to specify the task-name of an app or a task-run-id.

For example:

baur diff inputs calc.build 1 # compare the current inputs of the build task of the calc application with the ones of task-run 1
baur diff inputs 1 2 # compare the inputs of run 1 and 2
baur diff inputs calc.build shop.test # compare the current inputs of the build task of the calc app with the current inputs of the test task of the shop app

@fho I've started putting some test case scenarios together for this work and I would appreciate your input.
Can you please review the following and help me answer the questions in the Comments column.

app_one app_two^^ ? 1 How do we handle “run doesn’t exist”?
app_one|3 app_two|3 ? 1 How do we handle “run doesn’t exist for either app”?
app_one app_three ? 1 How do we handle “app doesn’t exist”?

When an app or run-id that was specified as parameter does not exist, always print an error message and exit.
We don't not need to do anything in the case. The user provided parameter invalid values.

Also, do you have a preferred format for the Output?

I had a format like the following in mind:
$ baur diff inputs random.build 1:

State   Path            Digest1         Digest2
D       main.go         sha384:123      sha384:12345
+       test.go                         sha384:2950
-       hello.go        sha384:6891

The State column indicates the type of change:

  • D - different digest
  • + the inputs of the run/task specified by the second parameter has a file that is not part of the inputs of the run/task of specified by the first parameter
  • - analogous, the inputs is part of the task/run specified by the first parameter but not of the task/run specified by the second parameter

The Digest1 is the digest from the inputs of the first parameter, Digest2 of the second parameter.

We could color the lines according to the status on the console.
This format could also be printed in csv format (--csv) when using the table formatter.

What do you think?

Thanks for your help
🎉
.

Thank you for your feedback, it all makes sense and should simply the implementation that I have been working on.
I have updated the test cases above, do you mind taking another look at them?
Using the existing table formatter should work well for this command 👍

@fho
Copy link
Collaborator Author

fho commented Oct 6, 2020

I have updated the test cases above, do you mind taking another look at them?

We misunderstand regarding the caret (^ syntax:
It is meant to work like in git. You still have to specify always 2 parameters to baur diff inputs.
app_one.build^ - references the last recorded run of the task
app_one.build^ - resolves the last recoded run before the last

That means for you examples:

Argument One Argument Two Output Exit code
app_one.build^ 1 show diff between run 2 and run 1 2
app_one.build^^ 1 Error: app_one.build^^ and 1 refer to the same task-run 1
app_one.build^^^ 1 Error: app_one.build^^^ does not exist, (only found 2 recorded runs in the database) 1
app_one.build^ app_two.compile show diff between run 2 and current files of app_two.compile 2
app_one.build^ Error: 2 parameter must be specified 1
app_one.build^^ Error: 2 parameter must be specified 1

The first implementation could also be done without the caret syntax.
It makes it only more user-friendly, we can add that syntax in a follow-up PR.

I would also suggest that was use different exit codes when baur exits because of an error and when diff inputs found differences.
Exit code for error:1,
If differences were found, exit code:2.

@fho
Copy link
Collaborator Author

fho commented Oct 6, 2020

@StephenDiligent btw, before you stumple over it: :-)
The table formatter currently does not work with colors currently.
It calculates the column width wrongly in some cases when the string contains ANSCI code sequencens (#205), easiest is to not use colors for now together with the formatter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants