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

[CLI] Determine future of CLI #267

Open
JoeZiminski opened this issue Nov 13, 2023 · 3 comments
Open

[CLI] Determine future of CLI #267

JoeZiminski opened this issue Nov 13, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@JoeZiminski
Copy link
Member

Currently the CLI basically wraps the API function-for-function. However, there is quite a lot of overhead with this, and it is not very clear for the user. Also, currently for some cases (e.g. get_next_sub_number()) not all underlying arguments are exposed. However, the CLI is fixed with a good test suite so should work okay for the time being, even if it is suboptimal.

With the TUI development, it is likely the CLI may not be used often. If this is the case, we can remove it, we can figure this out after proper release.

If it is continued, it should be overhauled. #125, #221 will need to be adressed. Related to #125, simply wrapping the API 1-1 isn't working and is becoming unweidly. If the CLI is not deleted, it should be designed as proper interface (i.e. by merging commands, then we can just have a configs, upload, download showers method) and remove attempt to 1:1 correspondance with API.

@JoeZiminski
Copy link
Member Author

With the removal of working top-level-folder in #344, many of the CLI functions were broken. It is not worth the time to fix these now. There is a NotImplementedError on the main function (except for the TUI). I would have liked to keep the original code underneath but SonarCloud was failing the linting because of it, so will paste here and add a note that this original code can be found prior to #344.

I am leaning towards removing all CLI code except for TUI launching, as it is undocumented and confusing if anyone finds. Then we can add it back in if able to properly support / it is required a lot.

        if "func" in args and str(args.func.__name__) == "make_config_file":
            warn = "ignore"
        else:
            warn = "default"

        warnings.filterwarnings(warn)  # type: ignore
        project = DataShuttle(args.project_name, print_startup_message=False)
        warnings.filterwarnings("default")

        if len(vars(args)) > 1:
            args.func(project, args)
        else:
            utils.print_message_to_user(
                f"Datashuttle project: {args.project_name}. "
                f"Add additional commands, see --help for details"
            )

@JoeZiminski
Copy link
Member Author

The CLI code that is defunct was removed in #347 to avoid having unused code in the publicly available codebase. However this PR is there in case we decide to add it back.

@JoeZiminski
Copy link
Member Author

JoeZiminski commented Apr 9, 2024

See relevant (currently closed, can reopen if using CLI) issues:
#125
#221
#291

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant