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

Abstract run_in_docker to a utility function #103

Conversation

jakeberesford-palmetto
Copy link
Collaborator

Pull request checklist

Before submitting your PR, please review the following checklist:

  • Consider adding a unit test if your PR resolves an issue.
  • All new and existing tests pass locally (palm test)
  • Lint (palm lint) has passed locally and any fixes were made for failures
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Breaking changes

  • Check if this pull request introduces a breaking change

What does this implement/fix? Explain your changes.

This will allow plugins to execute commands in docker during setup. We need this to reliably detect which dbt version a given project is using.

There is no functional change here, just a new abstraction to enable a change elsewhere.

Does this close any currently open issues?

Not directly, it's a step towards being able to resolve an open issue in palm-dbt

Where has this been tested?

MacOS + Docker v24
Tested with a dbt project using palm-dbt
Tested with a non-dbt project.

This will allow plugins to execute commands in docker during setup.
We need this to reliably detect which dbt version a given project is using.
@jakeberesford-palmetto jakeberesford-palmetto requested a review from a team as a code owner September 8, 2023 18:29
@jakeberesford-palmetto jakeberesford-palmetto requested review from emilypastewka, a user and david-jy-kim and removed request for a team September 8, 2023 18:29
This is useful when we need to do something with the output from Docker programmatically.
@jakeberesford-palmetto
Copy link
Collaborator Author

No merge because I'm still testing the capture output stuff to make sure it doesn't break anything. Should be safe.

This allows us to run commands in docker without outputting the command.
Maybe a little shady, but makes for a slicker DX in some cases.
@jakeberesford-palmetto
Copy link
Collaborator Author

I've been running this change for a few days now without any issues. Should be good-to-go!

Copy link
Collaborator

@mariahjrogers mariahjrogers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mushroom_dance

Copy link
Contributor

@norton120 norton120 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep that's what this does

@jakeberesford-palmetto jakeberesford-palmetto merged commit a2c4fe6 into palmetto:develop Oct 11, 2023
6 of 7 checks passed
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

Successfully merging this pull request may close these issues.

None yet

3 participants