kestra icon indicating copy to clipboard operation
kestra copied to clipboard

feat: support flow label values to be of data types different than string

Open Malaydewangan09 opened this issue 1 year ago • 4 comments

What changes are being made and why?

Allow flow label values to accept data types such as int, float, and boolean, in addition to strings. Complex data types, such as lists and maps, remain unsupported as label values. This change improves label flexibility and serialization while maintaining simple data type compatibility.

closes #5707

Malaydewangan09 avatar Oct 30 '24 14:10 Malaydewangan09

Yes, I'll add the test cases. Thanks!

Malaydewangan09 avatar Oct 31 '24 10:10 Malaydewangan09

@loicmathieu Given that I modified the label deserializer, Is it reasonable to include a test case such as this?

 @Test
void shouldHandleNonStringLabelValues() {
    Flow flow = Flow.builder()
        .labels(List.of(
            new Label("intLabel", "42"),
            new Label("boolLabel", "true"),
            new Label("floatLabel", "3.14"),
            new Label(Label.SYSTEM_PREFIX + "intSystemLabel", "100")
        ))
        .build();

    assertThat(labels, hasSize(3));
    assertThat(labels, hasItems(
        new Label("intLabel", "42"),
        new Label("boolLabel", "true"),
        new Label("floatLabel", "3.14")
    ));

Here that we are again passing strings only?

Malaydewangan09 avatar Oct 31 '24 12:10 Malaydewangan09

@Malaydewangan09 I'm not sure your test do anything. The best would be to:

  1. update the RuntimeLabelTest to use a label with a primitive type.
  2. Add a label to an existing flow in https://github.com/kestra-io/kestra/tree/develop/core/src/test/resources/flows/valids and either assert in an existing test that uses this flow or create a new test.

loicmathieu avatar Oct 31 '24 13:10 loicmathieu

Got it Thanks!

Malaydewangan09 avatar Oct 31 '24 13:10 Malaydewangan09