Skip to content

feat(graph-proxy): add mutation to create Triggers#1425

Open
JamesDoingStuff wants to merge 1 commit into
mainfrom
jg/trigger-mutation
Open

feat(graph-proxy): add mutation to create Triggers#1425
JamesDoingStuff wants to merge 1 commit into
mainfrom
jg/trigger-mutation

Conversation

@JamesDoingStuff

Copy link
Copy Markdown
Contributor

AP-1178
Adds a mutation to the graph that allows a Trigger to be created by specifying a ClusterTriggerTemplate to use. By default, triggers are named after the template and are in the events namespace. Name and visit can be passed as parameters in the mutation to override these (though random characters will still be appended to the name)

@JamesDoingStuff JamesDoingStuff self-assigned this Jul 3, 2026
/// The name of the Trigger
name: Option<String>,
/// The name of a ClusterTriggerTemplate that the Trigger is created from
#[graphql(name = "templateRef")]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

no a request to change as this is fine, but just so that you know, that snake_case to camelCase is done automatically async-graphql see: https://docs.rs/async-graphql/latest/async_graphql/derive.SimpleObject.html so this line is redundant

template_ref: String,
name: Option<String>,
visit: Option<VisitInput>,
) -> anyhow::Result<TriggerGQL> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

async_graphql will trung this return type into TriggerGQL! (a non-nullable type).

If you return a non-nullable type, and there is an error, GraphQL will not return null, so it must bubble up the error up the tree until it hit s a nullable type. If there is no nullable type between you and the root of the tree, it kills the whole query.

In async-graphql:

Option<T> becomes T that does not return an error.
Result<T> becomes T! that may return an error.
Result<Option<T>> becomes T that may return an error.

So, I believe this needs to return Result<Option<TriggerGQL>>.

see: https://diamondlightsource.slack.com/archives/C06N43M7JP3/p1764932359041159

let api: Api<Trigger> = Api::namespaced(client.clone(), &namespace);
let trigger = Trigger {
metadata: ObjectMeta {
generate_name: Some(format!("{}-", name.as_deref().unwrap_or(&template_ref))),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In GraphQL this turns into:

mutation createTrigger(templateRef: String!, name: String, visit: VisitInput): Option<T>

I guess generate_name and name work like Argo Workflows generateName and name? i.e. name is an exact name but generate name is <prefix>_randomstring?

If I call createTrigger(name:"foo") I would be confused when I get back an Trigger{name:"foo-bar"}.

I suggest to consider changing the behaviour or changing the name of the name parameter.

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