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

feat(pipeline/configuration): Updated pipeline level configuration that introduces a new property named metadata. #4808

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tonyg4864
Copy link
Contributor

SpEL processing pipeline.metadata so that it is resolved/ready when referenced.

…at introduces a new property named metadata.

SpEL processing pipeline.metadata so that it is resolved/ready when referenced.
@dbyron-sf
Copy link
Contributor

What's the motivation for this change?

@tonyg4864
Copy link
Contributor Author

tonyg4864 commented Dec 6, 2024

What's the motivation for this change?

  1. The original motivation is a CDEvents Pipeline level notification. I needed to send a Pipeline Level CDEvent notification with a payload that includes data from stages. Example : A Pipeline Level CDEvent Notification that includes data from all failed stages. Do not want to need to add a notification config to each specific stage to be able to accomplish this. Also I want the notification to be sent once at the end of the pipeline. Current functionality forces event notification at stage level leading to multiple events being sent every time a stage fails. The notification should be able to access Pipeline level constants/properties(metadata) avoiding the need to have successfully executed a "Evaluate Variables" stage.
  2. Second motivation is to be able to include pipeline wide properties/constants that can be referenced anywhere without having to run a "Evaluate Variables" stage.

@dbyron-sf
Copy link
Contributor

OK, I think I understand at least part of this, and I agree it's nice to be able to have a arbitrary blob make it from pipeline config to execution. And metadata is as good a word as any. I'm waffling on the need for a feature flag in case people already have metadata in their config and don't need/want/expect it to make it the execution. It seems harmless, but increasing the size of executions can cause pain.

The part I'm a little stuck on is the change to EvaluateVariablesStage. If you're saying that the idea is to be able to reference pipeline.metadata without an EvaluateVariablesStage, then why do we need a code change there?

@tonyg4864
Copy link
Contributor Author

"I'm waffling on the need for a feature flag in case people already have metadata in their config and don't need/want/expect it to make it the execution." If you think this is necessary/mandatory I can add it , let me know.

"It seems harmless, but increasing the size of executions can cause pain." I understand and I was also concerned about this. So far no issues in our workloads.

"The part I'm a little stuck on is the change to EvaluateVariablesStage. If you're saying that the idea is to be able to reference pipeline.metadata without an EvaluateVariablesStage, then why do we need a code change there?": It's the other way around. It is needed for the case where pipeline.metadata has SpEL expressions and pipeline.metadata is referenced in a EvaluateVariable stage. The changes in EvaluateVariableStage run pipeline.metadata through the SpEL processor so that it is resolved in case it is referenced somewhere in the stage.

@dbyron-sf
Copy link
Contributor

dbyron-sf commented Jan 6, 2025

"I'm waffling on the need for a feature flag in case people already have metadata in their config and don't need/want/expect it to make it the execution." If you think this is necessary/mandatory I can add it , let me know.

Yes please.

"The part I'm a little stuck on is the change to EvaluateVariablesStage. If you're saying that the idea is to be able to reference pipeline.metadata without an EvaluateVariablesStage, then why do we need a code change there?": It's the other way around. It is needed for the case where pipeline.metadata has SpEL expressions and pipeline.metadata is referenced in a EvaluateVariable stage. The changes in EvaluateVariableStage run pipeline.metadata through the SpEL processor so that it is resolved in case it is referenced somewhere in the stage.

Aaah, so what if something else references pipeline.metadata? Is this where we get back to needed nested SpEL expression evaluation? If so I'd rather pursue that than treating EvaluateVariablesStage as a special case.

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.

2 participants