Skip to content

Separate launcher cache from workflows#5019

Merged
Bubballoo3 merged 3 commits intomasterfrom
use-default-form-values-in-workflows-5017
Jan 28, 2026
Merged

Separate launcher cache from workflows#5019
Bubballoo3 merged 3 commits intomasterfrom
use-default-form-values-in-workflows-5017

Conversation

@Bubballoo3
Copy link
Contributor

Fixes #5017 by making sure launchers only use their cached values when run directly or when opening the form, not when run from a workflow or in the form editor. With these changes you should be able to

  1. Setup a launcher with environment variable VAR
  2. Set a default value like VAR=default
  3. Run the launcher (directly) and observe it using this default
  4. Run the launcher from the form, and provide a different value like VAR=cached
  5. Run the launcher directly, and see that it uses the cached value
  6. Run the launcher in a workflow, and see that it uses the default value
  7. Run the launcher directly, and see that the workflow did not write the cache.

By making sure workflows never read or write to the cache, you can always know that workflows will use default values shown in the form editor, and that running a launcher directly will always use cached values from the last time it was launched independently.

Remove raising an error when launcher is not found.
@johrstrom
Copy link
Contributor

I wonder if there's another way to do this. I'm not in love with this as it seems to violate a separation of concerns like the Launcher class is refitting itself for the Workflow. Though I'm not sure if we have a way around that at this time...

@Bubballoo3
Copy link
Contributor Author

Bubballoo3 commented Jan 27, 2026

There definitely could be a refactoring in the future, but maybe a better way of looking at it is that launcher caching as a feature only pertains to standalone launchers (to match behavior from batch connect forms), and this caching could be overridden by workflow option caching later on.

I think if we were to view caching as an essential feature of launchers, it still wouldn't account for using cached values in the form editor, where you expect that the values you input as defaults will remain the default values. To have these defaults replaced with cached values and then potentially saved as new defaults seems like inherently confusing behavior. This alone justifies having an alternative to get cache-free form data, which just so happens to be what makes the most sense in the workflows context.

The only piece that line of thought does not explain is preventing workflows from making new caches, which seems to be inherent in the expectation that cached values will be 'the last values you filled out in the form'. So until workflows have a form, caching the default values there (and overwriting your previous cache) is actually an intrusion of workflows into something that should be the sole purview of the Launcher class.

In summary, if we reframe caching as an 'add-on' to normal behavior, then it makes sense that other classes interacting with launchers would need a way to get the 'basic' or cacheless behavior.

@johrstrom
Copy link
Contributor

Yea maybe we just don't have any other option. Let's sleep on it and maybe we'll come back with something else.

@Bubballoo3
Copy link
Contributor Author

Yeah the only thing I can think of that might make this more clear is if we changed the affected methods so that cacheless behavior actually was the default, and launcher methods that want cached values would have to specify that explicitly.

I didn't take that route right away in order to treat the previous behavior as default, but the more I think on it cached values getting places that they shouldn't will probably cause more harm than the reverse. And it seems clear from this ticket that we haven't always been keeping caching in mind as stuff has grown around launchers, so it could be safer going forward to treat caching as opt-in for any launcher methods that require it.

class ClusterNotFound < StandardError; end

attr_reader :title, :id, :created_at, :project_dir, :smart_attributes
attr_reader :title, :id, :created_at, :project_dir, :cacheless_attributes, :smart_attributes
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I'm good with this, I'd just like a comment here about cacheless_attributes and how we're sort of end running around what we'd actually like to have.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added the comment when cacheless_attributes is defined in initialize. Does that capture everything you want it to? This still doesn't feel to me like too much of a workaround, so if you have anything you'd like to add that would be fine.

@johrstrom johrstrom self-requested a review January 28, 2026 19:21
Copy link
Contributor

@johrstrom johrstrom left a comment

Choose a reason for hiding this comment

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

LGTM - I also just re-ran that one flaky test.

@Bubballoo3 Bubballoo3 merged commit cc24511 into master Jan 28, 2026
47 of 48 checks passed
@github-project-automation github-project-automation bot moved this from Awaiting Review to Merged/Closed in PR Review Pipeline Jan 28, 2026
@Bubballoo3 Bubballoo3 deleted the use-default-form-values-in-workflows-5017 branch January 28, 2026 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Default values for launcher forms do not persist

3 participants