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

fix(standardise-validate): allow empty string from Argo for start and end dates TDE-1346 #1286

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amfage
Copy link
Contributor

@amfage amfage commented Feb 13, 2025

Motivation

Argo Workflows supplies single quotes for optional parameters, which are not valid dates. These are handled later in the standardise_validate.py code.

Modifications

Allow an empty string to be accepted by the command line parser for the start and end dates.

Verification

Added unit tests. Tested from Argo Workflows using a PR container.

@amfage amfage requested a review from a team as a code owner February 13, 2025 03:01
@amfage amfage added the container Publish a container label Feb 13, 2025
@amfage amfage changed the title fix(standardise-validate): allow empty string '' from Argo for start and end dates TDE-1346 fix(standardise-validate): allow empty string from Argo for start and end dates TDE-1346 Feb 13, 2025
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.

A NIT and subjective comment

@@ -71,3 +74,17 @@ def test_coalesce_nothing() -> None:
single_item = ""
coalesced_list = coalesce_multi_single(multi_items, single_item)
assert coalesced_list == []


def test_valid_date_empty_string() -> None:
Copy link
Collaborator

@paulfouquet paulfouquet Feb 13, 2025

Choose a reason for hiding this comment

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

You can instead create doctests for this kind of unit tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
container Publish a container
Development

Successfully merging this pull request may close these issues.

2 participants