Skip to content

Conversation

sbliven
Copy link
Member

@sbliven sbliven commented May 22, 2025

Description

This refactors the switch action considerably to support dataset and job properties in more circumstances, fixing #1927.

Configuration Changes:

  • Remove scope from the switch action. property now references the full context. Scopes should be included in the property, eg request.jobParams or datasets[*].datasetlifecycle (note the wildcard json path, required for arrays).
  • Add phase configuration to control whether the switch applies during validate, perform, or all phases.
  • Rename performJob to perform in the log action to match the switch phase

Behavioral Changes:

  • Fall through to the default case if the property doesn't match anything (this used to be an error)
  • Datasets can be accessed in create jobs during the perform phase
  • Job properties can be accessed in update jobs during the validate phase

Motivation

Previously the switch property always applied to both phases, and threw an error if the property was not found. The validate phase only was able to switch over request properties. This was particularly limiting for update operations, since the request DTO doesn't overlap much with the job, so it wasn't possible to access most properties (eg any datasets). This provides significantly more flexibility.

Fixes

Changes:

  • jobConfig.yaml should be updated. The switch and log actions have different options.

Tests included

  • Included for each change/fix?
  • Passing?

Documentation

  • swagger documentation updated (required for API changes)
  • official documentation updated

official documentation info

SciCatProject/documentation#75

sbliven added 2 commits May 21, 2025 17:10
Configuration & Behavioral Changes:

- Remove `scope` from the switch action. `property` now references the
  full context. Scopes should be included in the property, eg 
  `request.jobParams` or `datasets[*].datasetlifecycle`.
- Add `phase` configuration to control whether the switch applies
  during validate, performJob, or all phases.
- Fall through to the default case if the property doesn't match anything
- Datasets can be accessed in create jobs during the performJob phase

Implementation Changes:
- Remove SwitchCreateJobAction. SwitchJobAction is used for both update
  and create operations.
- Type checks for SwitchJobActionOptions is more rigorous
- Update unit tests
Importantly, this makes dataset properties available to update jobs
in the validate phase. It does have the downside of potential confusion
where `job.statusCode` (and other updatable fields) could change values
between the validate and perform phases, but actions should then use
`request.statusCode`, which will have a stable value.
@sbliven
Copy link
Member Author

sbliven commented May 22, 2025

I am working on updating docs.
I may rename the phases based on discussion in #1927.
Otherwise it's ready for review

@despadam
Copy link
Member

For the naming of the phases, I would still vote for validate / perform (or performJob if you would prefer to keep it as it currently is).
Regarding the documentation, are you referring to this PR?

@sbliven
Copy link
Member Author

sbliven commented May 28, 2025

I like @despadam's suggestion of validate/perform phases. I've updated to use those.

SciCatProject/documentation#73 corresponds to #1914, which has already merged. I need to incorporate your changes and merge that. Afterwards I will add documentation of the #1931 changes.

@sbliven sbliven merged commit beafb7e into SciCatProject:master May 28, 2025
15 checks passed
@sbliven sbliven deleted the fix-1927 branch May 28, 2025 11:17
@despadam despadam mentioned this pull request Jun 3, 2025
4 tasks
github-merge-queue bot pushed a commit that referenced this pull request Jun 3, 2025
<!--
Follow semantic-release guidelines for the PR title, which is used in
the changelog.

Title should follow the format `<type>(<scope>): <subject>`, where
- Type is one of:
build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test|BREAKING
CHANGE
- Scope (optional) describes the place of the change (eg a particular
milestone) and is usually omitted
- subject should be a non-capitalized one-line description in present
imperative tense and not ending with a period

See
https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines
for more details.
-->

## Description
<!-- Short description of the pull request -->
- #1954
- #1913
- #1931
- #1934

## Motivation
<!-- Background on use case, changes needed -->
- #1937
- #1776
(topic 2)
- #1927

## Fixes
<!-- Please provide a list of the issues fixed by this PR -->

## Changes:
<!-- Please provide a list of the changes implemented by this PR -->

## Tests included

- [ ] Included for each change/fix?
- [ ] Passing? <!-- Merge will not be approved unless tests pass -->

## Documentation
- [ ] swagger documentation updated (required for API changes)
- [ ] official documentation updated

### official documentation info
<!-- If you have updated the official documentation, please provide PR #
and URL of the updated pages -->
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