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

Add pre/post hooks for std tests #2233

Merged
merged 2 commits into from
Mar 4, 2021
Merged

Conversation

sherifnada
Copy link
Contributor

@sherifnada sherifnada commented Feb 27, 2021

What

Adds pre/post hooks that can be run for file-based standard tests. This allows configuring inputs to the test and cleaning them up afterwards.

The first use case will be in the Mongodb PR #2125 to init and tear down the dockerized DB. See an example usage https://github.com/narrative-bi/airbyte/pull/2

'--spec', "${targetMountDirectory}/${config.specPath}",
'--config', "${targetMountDirectory}/${config.configPath}",
]
config.prehook.call()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not put this in the try? Seems like the prehook may do some things that need to be cleaned up even if it fails.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am getting super nervous by the amount of logic we put in gradle at the moment.

What is the longer term vision here?

Copy link
Contributor

Choose a reason for hiding this comment

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

why isn't his running in the docker container that you're running instead of in gradle / outside of a controlled docker environment?

Copy link
Contributor Author

@sherifnada sherifnada Mar 3, 2021

Choose a reason for hiding this comment

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

The docker container run here executes tests which are configured via files. In case we need to say, run and stop a docker container in pre/post hooks (in this case used for Mongo testing), this seems like the most ergonomic way to declare pre/post hooks which start/stop the DB image assuming we don't have language specific bindings. These hooks are better used to run docker images, not arbitrary code. I could change the interface to say, take as input a docker image instead of run freeform code, but I think this gets wonky if the user wants to pass options to the docker command.

We could declare these in language-specific bindings (similar to what we do in python) but this is really meant as an escape hatch for languages for which we don't have those bindings (in this case Ruby).

@sherifnada sherifnada merged commit d382c87 into master Mar 4, 2021
@sherifnada sherifnada deleted the sherif/add-stdtest-hooks branch March 4, 2021 00:34
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.

4 participants