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

Make namespace and flow optional when using keep_original_source = true #79

Open
tchiotludo opened this issue Aug 18, 2023 · 7 comments
Assignees
Labels
area/backend Needs backend code changes enhancement New feature or request kind/cooldown Great candidate for the cooldown period

Comments

@tchiotludo
Copy link
Member

right now, we could read it from the yaml like that

resource "kestra_flow" "demo_flows" {
  for_each = fileset(path.module, "kestra/io.kestra.demo/*.yml")
  flow_id = yamldecode(templatefile(each.value, {}))["id"]
  namespace = yamldecode(templatefile(each.value, {}))["namespace"]
  content = templatefile(each.value, {})
  keep_original_source = true
}

but we are able to read it directly from the yaml since the id and namespace are mandatory

@tchiotludo tchiotludo added the enhancement New feature or request label Aug 18, 2023
@github-project-automation github-project-automation bot moved this to Backlog in All issues Aug 18, 2023
@tchiotludo tchiotludo moved this from Backlog to Ready in All issues Aug 18, 2023
@aballiet
Copy link
Contributor

Or make the opposite : use terraform fields and internally generate this mandatory part of the YAML.
Makes sense to define these values with TF params to me.

WDYT ?

@tchiotludo
Copy link
Member Author

I've an issue, the goal keep_original_source is to kept exact source of the flow, if the people want to add comment everywhere (at the beginning, end, between namespace and id properties), it must be able to it, when we unserialize, serialize a yaml string, most of parsers will break that (remove comment, changing quote, ...), so it appear to be not the good way

@aballiet
Copy link
Contributor

aballiet commented Aug 18, 2023

Okey, and what about doing it in the API call : https://kestra.io/docs/api-guide/#post-/api/v1/flows.

Using values from content for everything except id and namespace if var provided using TF ressource, otherwise use all values from content.

if yamlSourceCode == true {
r, reqErr := c.yamlRequest("POST", "/api/v1/flows", stringToPointer(d.Get("content").(string)))
if reqErr != nil {
return diag.FromErr(reqErr.Err)
}
errs := flowSourceApiToSchema(r.(map[string]interface{}), d)
if errs != nil {
return errs
}
return diags
} else {
body, err := flowSchemaToApi(d)
if err != nil {
return diag.FromErr(err)
}
r, reqErr := c.request("POST", "/api/v1/flows", body)
if reqErr != nil {
return diag.FromErr(reqErr.Err)
}
errs := flowApiToSchema(r.(map[string]interface{}), d)
if errs != nil {
return errs
}
return diags
}

tldr : not having to unserialize, serialize a yaml string

@aballiet
Copy link
Contributor

In the end it's more a question about how we see the terraform provider.

If no logic at all should be implemented within the provider then we go with only content & keep_original_source params.

Otherwise we could also have flow_id, namespace and some other future fields.

@anna-geller anna-geller added this to the v0.15.0 milestone Dec 4, 2023
@anna-geller anna-geller removed this from All issues Dec 12, 2023
@anna-geller anna-geller modified the milestones: v0.15.0, v0.17.0 Jan 16, 2024
@Skraye Skraye self-assigned this May 22, 2024
@Skraye Skraye modified the milestones: v0.17.0, v0.18.0 May 30, 2024
@github-project-automation github-project-automation bot moved this to Backlog in Issues Jun 10, 2024
@Skraye
Copy link
Member

Skraye commented Jul 17, 2024

As discuss with Ludo long times ago, this would be tricky, so we decided that the change will be done when we will fully change the API to create flows

@Skraye Skraye removed this from the v0.18.0 milestone Jul 17, 2024
@aballiet
Copy link
Contributor

As discuss with Ludo long times ago, this would be tricky, so we decided that the change will be done when we will fully change the API to create flows

OK yes. Can't wait for this. Do you have an idea when it will be released?

@Skraye
Copy link
Member

Skraye commented Jul 25, 2024

@aballiet We just deprecated methods using JSON to create/edit flow, so I can't really tell

@anna-geller anna-geller added area/backend Needs backend code changes kind/cooldown Great candidate for the cooldown period labels Sep 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/backend Needs backend code changes enhancement New feature or request kind/cooldown Great candidate for the cooldown period
Projects
Status: Backlog
Development

No branches or pull requests

4 participants