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

"12/16/2021: Make it easier to construct simple Bash commands" #412

Open
mothslaw opened this issue Dec 16, 2021 · 0 comments · May be fixed by #413
Open

"12/16/2021: Make it easier to construct simple Bash commands" #412

mothslaw opened this issue Dec 16, 2021 · 0 comments · May be fixed by #413

Comments

@mothslaw
Copy link
Contributor

Is your feature request related to a problem? Please describe.
A common source of bugs has to do with constructing and formatting Bash commands.

For example, let's say your plugin wants to make a copy of an existing config file on a remote host, and this new copy should have a name that includes the name of a VDB.

This might cause you to write plugin code like this:

command = "cp {} {}_config.txt".format(repository.config_template_path, virtual_source.parameters.name)
libs.run_bash(cx, command)

You might test this and see that it "works", and so you release your plugin to customers.

However, someday, some user will come along and name their VDB "Alice's Data". Then, the above code will not work, because Bash will see that single-quote character and will complain that it is unmatched by another quote character.

So, plugins have to worry about quoting and escaping all of these strings, as they assemble their Bash command lines. This is tedious and error prone.

This feels like something that the Platform Libraries should be doing.

Describe the solution you'd like
Today, we have a single run_bash function where the caller says "Here is the exact string that should be passed to Bash". It works well for hardcoded commands, and for multiline scripts. But, it's not the best for parameterized commands.

I think we should provide a helper function that will construct these strings, for the simple case where:

  • The caller wants to run one single command like rm or /opt/hana/bin/db_init or ps.
  • The caller has Python strings that they want to pass to the command as/is, without having to worry about quoting/escaping any special characters that Bash would otherwise interpret.

Something like this:

command = libs.construct_bash_command("cp", repository.config_template_path, "{}_config.txt".format(virtual_source.parameters.name))
libs.run_bash(cx, command)

Describe alternatives you've considered
Another option here would be a run_executable function, which cuts Bash out of the picture entirely:

libs.run_executable("cp", repository.config_template_path, "{}_config.txt".format(virtual_source.parameters.name))

This has two drawbacks, though:

  1. It's a lot more work, as it would required engine/backend support. My preferred solution only requires a new helper function on the SDK side only.
  2. It would only work with on-disk executable files, and would not work with Bash shell builtins.

Additional context
Most of Delphix's in-house plugins have worked around this problem by writing their own function, usually called quote_for_bash that handles part of this work. So, they would write something like this:

base_command = quote_for_bash("cp")
arg1 = quote_for_bash(repository.config_template_path)
arg2 = quote_for_bash({}_config.txt".format(virtual_source.parameters.name))
command = "{} {} {}".format(base_command, arg1, arg2)
libs.run_bash(command)

This works well enough, but it's quite cumbersome, and sometimes people forget to do it.

Plus, In general, if there's code that lots of plugins separately implement, then that's likely something that the SDK should provide for them.

mothslaw added a commit to mothslaw/virtualization-sdk that referenced this issue Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant