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

Migration tool CLI redesign #56

Closed
happyRip opened this issue Nov 8, 2022 · 3 comments · Fixed by #70 or #71
Closed

Migration tool CLI redesign #56

happyRip opened this issue Nov 8, 2022 · 3 comments · Fixed by #70 or #71

Comments

@happyRip
Copy link
Member

happyRip commented Nov 8, 2022

Summary

lorawan-stack-migrate tool could use a different approach, mainly the flag system.

Why do we need this?

The migration has been growing and the flag system is getting out of hand.

The way we implement flags currently does not support adding flags specific to a command (applications, device) or sources. This creates a lot of fuzz in the flags we present to the user that are not relevant for the current use case and it's only going to get worse:
image

An example of this is the flag app-id is not needed when using applications command, because the app-id is supplied in the command itself.

Other than that firefly source wouldn't need applications command as much as organizations as to what I've seen that is used more often to organize devices on the platform. Because of that being able to define commands per source would be beneficial as well.

What is already there? What do you see now?

We have a working CLI.

What is missing? What do you want to see?

Flexibility and structure of the CLI.

Environment

n/a

How do you propose to implement this?

Use a well developed CLI library like spf13/cobra and rethink the way we implement stuff.

How do you propose to test this?

Test migrating devices with new CLI.

Can you do this yourself and submit a Pull Request?

I can implement this myself, but it would require help in design from a more experienced developer.

@happyRip happyRip added the needs/triage We still need to triage this label Nov 8, 2022
@NicolasMrad NicolasMrad added discussion and removed needs/triage We still need to triage this labels Nov 15, 2022
@NicolasMrad NicolasMrad added this to the 2022 Q4 milestone Nov 15, 2022
@adriansmares
Copy link
Contributor

Indeed there are some improvements that we can do to make the flags a bit more bearable:

  • Separate the applications and device flag sets on create. Right now a single flag set is provided to the source Create method, and then every source adds all of the flags. An alternative here is that Create receives two flag sets, and adds the flags required by each command.

The problem with the two tier structure (tti-lw-migrate ttnv3 devices ...) is that most flags libraries are not actually built to support multiple tiers of commands. With that being said, I understand the need for this.

We can programmatically create commands such as <source name>-devices and <source name>-applications and hide the original devices / applications commands. The added advantage here is that we can add the flags only to the appropriate commands.

For legacy reasons I don't think we should remove the old devices/applications command necessarily, but they should be marked as Hidden instead.

@happyRip
Copy link
Member Author

We could add a command like ttn-lw-cli use that would configure msot flags of the sources. If the --source flag would be configured with this, there could be no need for multiple tiers of commands since any following calls after e.g. to migrate from community:

$ ttn-lw-migrate use eu1.cloud.thethings.network

would be pre-configured and only need to be called with minimal input

@NicolasMrad NicolasMrad modified the milestones: 2022 Q4, v0.9.1 Nov 29, 2022
@adriansmares
Copy link
Contributor

That may also work, but in general I still recommend separating the flag sets.

I think we should this use command as a settings generator, that would generate the env variables, such that the user can to ttn-lw-migrate use eu1.cloud.thethingsnetwork > file and source file maybe.

Please consider doing an iteration based on what we discussed here (separation of flags and use generation) and let's discuss on a PR.

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