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.

Sign up for GitHub

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

Jump to bottom

tink-cli should parse template name from the template itself #377

Closed
mmlb opened this issue Dec 4, 2020 · 5 comments · Fixed by #385
Closed

tink-cli should parse template name from the template itself #377

mmlb opened this issue Dec 4, 2020 · 5 comments · Fixed by #385
Assignees
@parauliya
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@mmlb
Copy link
Contributor

mmlb commented Dec 4, 2020

Why do we have a template name cli arg when name is a require template field? tink-cli should use the name in the template. Doing otherwise can lead to lots of confusion.
@mmlb mmlb added the kind/bug Categorizes issue or PR as related to a bug. label Dec 4, 2020
@thebsdbox thebsdbox added kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Dec 10, 2020
@parauliya parauliya self-assigned this Dec 10, 2020
@gianarb
Copy link
Contributor

gianarb commented Dec 10, 2020

@parauliya can you tell us how you want to fix this issue first?
Thanks

@parauliya
Copy link
Contributor

parauliya commented Dec 10, 2020 •
edited
Loading

first

Sure @gianarb .
I am thinking of removing the requirement of name from the tink template create command and for the database I will take that field from the template data.
What do you think?

@gianarb
Copy link
Contributor

gianarb commented Dec 10, 2020

I think it is created and if other commands have the same issue, please remove the flag as well!

parauliya pushed a commit to parauliya/tink that referenced this issue Dec 11, 2020
Fix tinkerbell#377: Removing name flag from the 'tink template crea…
afde2c4 
…te/update' command in tink-cli

Signed-off-by: parauliya <[email protected]>
@parauliya parauliya mentioned this issue Dec 11, 2020
Merged
3 tasks
parauliya pushed a commit to parauliya/tink that referenced this issue Dec 11, 2020
Fix tinkerbell#377: Removing name flag from the 'tink template crea…
764f25b 
…te/update' command in tink-cli

Signed-off-by: parauliya <[email protected]>
gauravgahlot pushed a commit to parauliya/tink that referenced this issue Dec 18, 2020
@gauravgahlot
Fix tinkerbell#377: Removing name flag from the 'tink template crea…
28250b4 
…te/update' command in tink-cli

Signed-off-by: parauliya <[email protected]>
gauravgahlot pushed a commit to parauliya/tink that referenced this issue Dec 23, 2020
@gauravgahlot
Fix tinkerbell#377: Removing name flag from the 'tink template crea…
b0a5672 
…te/update' command in tink-cli

Signed-off-by: parauliya <[email protected]>
gauravgahlot pushed a commit to parauliya/tink that referenced this issue Dec 24, 2020
@gauravgahlot
Fix tinkerbell#377: Removing name flag from the 'tink template crea…
78215b3 
…te/update' command in tink-cli

Signed-off-by: parauliya <[email protected]>
gauravgahlot pushed a commit to parauliya/tink that referenced this issue Dec 29, 2020
@gauravgahlot
Fix tinkerbell#377: Removing name flag from the 'tink template crea…
949fa92 
…te/update' command in tink-cli

Signed-off-by: parauliya <[email protected]>
gauravgahlot pushed a commit to parauliya/tink that referenced this issue Jan 4, 2021
@gauravgahlot
Fix tinkerbell#377: Removing name flag from the 'tink template crea…
6843a52 
…te/update' command in tink-cli

Signed-off-by: parauliya <[email protected]>
gauravgahlot pushed a commit to parauliya/tink that referenced this issue Jan 4, 2021
@gauravgahlot
Fix tinkerbell#377: Removing name flag from the 'tink template crea…
fe1ced4 
…te/update' command in tink-cli

Signed-off-by: parauliya <[email protected]>
gauravgahlot pushed a commit to parauliya/tink that referenced this issue Jan 5, 2021
@gauravgahlot
Fix tinkerbell#377: Removing name flag from the 'tink template crea…
2af794d 
…te/update' command in tink-cli

Signed-off-by: parauliya <[email protected]>
gauravgahlot pushed a commit to parauliya/tink that referenced this issue Jan 12, 2021
@gauravgahlot
Fix tinkerbell#377: Removing name flag from the 'tink template crea…
6ba1bca 
…te/update' command in tink-cli

Signed-off-by: parauliya <[email protected]>
gauravgahlot pushed a commit to parauliya/tink that referenced this issue Jan 21, 2021
@gauravgahlot
Fix tinkerbell#377: Removing name flag from the 'tink template crea…
d667759 
…te/update' command in tink-cli

Signed-off-by: parauliya <[email protected]>
@mergify mergify bot closed this as completed in #385 Jan 21, 2021
mergify bot added a commit that referenced this issue Jan 21, 2021
@mergify
Fix #377: Removing name flag from the tink template create/update com…
26abf73 
…mand (#385)

## Description

This PR will remove the `name` flag from the `tink template create/update` command in tink-cli.

## Why is this needed

Fixes: #377 

## How Has This Been Tested?

This has been tested manually over the vagrant setup.

## How to migrate?

**Current Behaviour**:
At present, the `tink template create -n <name>` allows users to provide a name to a workflow template. A user can also update this name using the `tink template update -n <new-name>` command.

The workflow template also has the `name` field. So, a workflow is getting itss name from two different and places and this can be confusing to users.
```
version: "0.1"
name: hello_world_workflow
global_timeout: 6
tasks:
  - name: "hello world"
    worker: "{{.device_1}}"
    actions:
      - name: "hello_world"
        image: hello-world
        timeout: 60
```

**New Behaviour**:
With this PR, `tink template create` and `tink template update` CLI will _not_ accept a name for the template to be created or updated. Instead, the template name will be picked from the `name` field in the template definition. 

The existing users need to run the `tink template update <id> --file <template-file>` command. This will update the name of each template as per the value of `name` field in the template definition.

## Checklist:

I have:

- [ ] updated the documentation and/or roadmap (if required)
- [ ] added unit or e2e tests
- [x] provided instructions on how to upgrade
@moadqassem
Copy link
Member

@gianarb Would be cool if you can update the documentation where the template name flag isn't mentioned during the workflow creation. I added an issue to track it. #448. But actually, I find having a --name is still useful. If the --name was provided then you should override the template with the flag value.

@gianarb
Copy link
Contributor

gianarb commented Mar 4, 2021

Hey @moadqassem the documentation is based on the last tagged version of the sandbox. It still supports --name flag and that's why we didn't update it yet.

The PR you opened to the documentation will be merged as long as other updates as soon as we are ready to bump sandbox! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@parauliya parauliya

Labels
kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #377: Removing name flag from the tink template create/update command parauliya/tink
5 participants
@mmlb @gianarb @thebsdbox @moadqassem @parauliya