Skip to content

Conversation

@chrispine
Copy link

@chrispine chrispine commented Aug 26, 2020

By changing the JSON marshalling of Task to ignore the Template, this makes it so that changes to the template (assuming no changes to steps: or on:) will serialize to the same JSON, so the previous run is found in the cache.

We then reuse the diff from the cache, and otherwise use the new spec.

Testing: I tested this manually with the hello world campaign. I think everything worked correctly, and that my errors were unrelated to this feature, but I'd really appreciate someone else running a campaign and verifying that this works for them, too.

Closes #13172

Copy link
Contributor

@LawnGnome LawnGnome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an elegant solution. (Actually, quite a bit more elegant than I expected it to be when we talked about it!)

Since it took me a couple of reads to get why this was OK, I'd love a touch more commentary in the cache path, but otherwise I think this is good to go. 👍

Copy link
Contributor

@mrnugget mrnugget left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this was ... less hairy than I thought it would be. Well done! 👏

Agree with Adam that a comments would be cool

@chrispine
Copy link
Author

Credit for elegance of the solution goes entirely to Erik. 😄

Copy link
Contributor

@LawnGnome LawnGnome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, but I am interested in @mrnugget's thoughts on the multiple commit scenario, at least in theory. (I think what's in place right now is fine.)

@mrnugget
Copy link
Contributor

Left my thoughts here #282 (comment) (and linking to them because it's easy to miss in GitHub's PR view).

I'm going to merge this now so it's in main and I can use it along with the other fixes 😄

@mrnugget mrnugget merged commit f9ea964 into main Aug 28, 2020
@mrnugget mrnugget deleted the cp/campaigns-respect-valid-diff branch August 28, 2020 07:50
scjohns pushed a commit that referenced this pull request Apr 24, 2023
)

* Do not re-execute campaign spec steps when the diff is still valid #13172

* improved comments

Co-authored-by: Chris Pine <cpine@newrelic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not re-execute campaign spec steps when the diff is still valid

4 participants