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

Don't save step context during restart #27

Merged
merged 1 commit into from
Jan 7, 2022

Conversation

timja
Copy link
Member

@timja timja commented Dec 22, 2021

#18 (comment)

I've tested this manually and code paths work correctly and it's no longer saved in the build.xml

Bit convoluted though =/

@timja timja added the bug label Dec 22, 2021
@timja timja requested a review from jglick December 22, 2021 22:55
FlowExecutionOwner.Executable executable = (FlowExecutionOwner.Executable) this.run;
FlowExecutionOwner flowOwner = executable.asFlowExecutionOwner();
if (flowOwner != null) {
node = flowOwner.get().getNode(id);
Copy link
Member

Choose a reason for hiding this comment

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

null check IIRC

Copy link
Member Author

Choose a reason for hiding this comment

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

.get() is non null, the node is checked for null on line 121

Copy link
Member

Choose a reason for hiding this comment

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

Oh right, I meant to say that you could consider calling getOrNull so as to fall back to TaskListener.NULL in more edge conditions. Not sure it would be helpful though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah right, done in the last commit, and simplified a bit

@timja timja requested a review from jglick January 7, 2022 18:57
FlowExecutionOwner.Executable executable = (FlowExecutionOwner.Executable) this.run;
FlowExecutionOwner flowOwner = executable.asFlowExecutionOwner();
if (flowOwner != null) {
node = flowOwner.get().getNode(id);
Copy link
Member

Choose a reason for hiding this comment

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

Oh right, I meant to say that you could consider calling getOrNull so as to fall back to TaskListener.NULL in more edge conditions. Not sure it would be helpful though.

@timja timja force-pushed the dont-save-step-context branch from 76233b7 to c2267a6 Compare January 7, 2022 20:07
@timja timja enabled auto-merge (squash) January 7, 2022 20:17
@timja timja merged commit 010b1c5 into jenkinsci:master Jan 7, 2022
@timja timja deleted the dont-save-step-context branch January 7, 2022 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants