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

!cmd hook shell option should be optional #1482

Open
dmarra opened this issue Jul 1, 2024 · 4 comments
Open

!cmd hook shell option should be optional #1482

dmarra opened this issue Jul 1, 2024 · 4 comments

Comments

@dmarra
Copy link

dmarra commented Jul 1, 2024

Subject of the issue

Currently, the !cmd hook has two forms:

  1. simple string
  2. object with keys run and shell

The second form is more useful, as it allows revolvers to be leveraged to construct commands. However, if one wants to use use the default shell (in other words, setting executable=None in the subprocess.check_call(); they must use the first form, as in the second form both options are required.

In my case, I am running Sceptre in windows, within gitbash locally. I am using exported environment vars to set my profile. My colleagues are on different macs and linux; and production deployment would likely be in ubuntu. The type of command I am trying to run:

before_update:
    - !cmd: "aws s3 cp ../some_local_file.py s3://bucket-from-another-stack-output/"

The above works, but it is hard coded and therefore less than ideal. The following would be better:

before_update:
     - !cmd 
         run: !join
            - " "
            - - "aws s3 cp ../some_local_file.py"
               - !stack_output_external another_stack::BucketLocation               

This fails though because I left out shell. same command, but now shell is required. A few problems though:

  1. I can't set this in a cross-platform way
  2. setting it will run the command in a different shell, which may not inherit my exported environment var (or might not even work the same)

In a case like this, I really just want to leverage the power of the resolver to construct my command without worrying about which shells to invoke to do so. The current implementation is too rigid. It would be much friendlier if shell was an optional argument, such that one can leverage revolvers whether they want to use a different shell or not.

Your environment

  • 4.4.2
  • 3.12.3
  • Windows 11

Steps to reproduce

Add something like this to a stack config:

before_update:
     - !cmd 
         run: !join
            - " "
            - - "echo 'hello"
               - "world'"               

Expected behaviour

It should behave the same way it does as if you simply pass it a simple string; no shell executable gets set, but the command is constructed via the revolvers used

Actual behaviour

You will get an exception because the error checking in the hook couples run and shell, even though there is no reason why they need to be coupled

@alex-harvey-z3q
Copy link
Contributor

@dmarra The proposal makes sense to me although the author of that plugin has left us! Do you think you'd be able to send in a PR?

@dmarra
Copy link
Author

dmarra commented Jul 8, 2024

I'd love to! The change is quite easy to make. The challenge for me is running through the integration tests (and the time to get set up)

@alex-harvey-z3q
Copy link
Contributor

@dmarra Send in the PR and we can help you with the integration tests.

@dmarra
Copy link
Author

dmarra commented Aug 9, 2024

@alexharv074 very well will do. Appreciate that.

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

No branches or pull requests

2 participants