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

refactor: templates for standardise-validate and create-collection topo-imagery commands TDE-1346 #970

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

Conversation

amfage
Copy link
Contributor

@amfage amfage commented Feb 7, 2025

Motivation

The standardise-validate and create-collection commands need to be used by multiple workflows, as we are now publishing multiple products such as a National DEM and National Hillshades.

Modification

Create separate WorkflowTemplates for standardise-validate and create-collection.
Update current WorkflowTemplates national-dem and imagery-standardising that use these templates.

Checklist

  • Tests updated
  • Docs updated
  • Issue linked in Title

@amfage amfage requested review from a team as code owners February 7, 2025 02:13
@amfage amfage requested review from blacha, schmidtnz, MDavidson17 and paulfouquet and removed request for MDavidson17 February 7, 2025 02:13
Copy link
Collaborator

@paulfouquet paulfouquet left a comment

Choose a reason for hiding this comment

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

The national-dem WorkflowTemplate is also using this two templates. It would be great to point it to these new templates.

@amfage amfage marked this pull request as draft February 9, 2025 20:36
@amfage amfage marked this pull request as ready for review February 13, 2025 03:22

container:
image: '019359803926.dkr.ecr.ap-southeast-2.amazonaws.com/topo-imagery:{{=sprig.trim(inputs.parameters.version_topo_imagery)}}'
args:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is missing --capture-dates?

- '{{=sprig.trim(inputs.parameters.historic_survey_number)}}'
- '--lifecycle'
- '{{=sprig.trim(inputs.parameters.lifecycle)}}'
- '--add-title-suffix'
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be optional. For example, national DEM does not want the suffix in the title.

spec:
templateDefaults:
container:
imagePullPolicy: Always
Copy link
Contributor

Choose a reason for hiding this comment

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

Not specific to this PR, as this is something we do in most (if not all) our yaml files.
I'm wondering why we specify the imagePullPolicy on templateDefaults level with empty string for image instead of setting the policy where the actual container / image gets set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, it is in the spec so it applies to all containers within a workflowtemplate.
In this instance, it's just there in case this workflowtemplate is run standalone and not called from another workflowtemplate.

@amfage amfage marked this pull request as draft February 13, 2025 20:41
@amfage amfage marked this pull request as ready for review February 14, 2025 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants