search

skeema / tengo

Go La Tengo: a MySQL automation library
Apache License 2.0
27 stars 18 forks source link

Add SourceString and Source methods to DockerizedInstance #15

Closed mhemmings closed 3 years ago

mhemmings commented 3 years ago

Added the ability to source a string rather than a file. Also exposed the new method Source which takes an io.Reader allowing for any implementation.

Ideally, SourceSQL would be renamed to SourceFile, but I didn't want to break backwards compatibility.

Let me know what you think!

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.02%) to 94.392% when pulling 2036059e1483c43d188a0ed36ab5d70776609528 on mhemmings:main into c932fb4d9fad1d3852f8ab4e7c1d0845c1d25b9c on skeema:main.

evanelias commented 3 years ago

Thank you for the PR. My apologies, I apparently forgot to copy the CONTRIBUTING.md from the main Skeema repo over here, but please see https://github.com/skeema/skeema/blob/main/CONTRIBUTING.md. In particular if you could please describe the specific use-case motivating the new methods, that would be very helpful in evaluating the PR.

Sorry to put up barriers to contribution, but this is a bootstrapped effort and the amount of time I can devote to open source is extremely limited for the foreseeable future. The majority of my Skeema-related efforts are currently going towards proprietary commercial products, as that will be required in order for the open source projects to be sustainable.

mhemmings commented 3 years ago

No problem.

So we're wanting to use tengo for some projects diffing sql, easily spinning up and modifying Dockerised MySQL in tests etc.

We need to ability to spin up Docker using SQL stored as strings instead of files. For example, when they're programmatically generated. This PR adds the SourceString method which allows this.

The second method Source is what is used in the underlying implementations of SourceString and SourceSQL. It's exported as it may be useful to others, and it's also a much more idiomatic approach in Go to pass an io.Reader rather than a filename. Saying that, SourceSQL is still retained for backwards compatibility.

evanelias commented 3 years ago

Ordinarily for dynamically-generated DDL and DML in tests, I'd recommend running individual queries through the driver (see e.g. this construct for test helper methods). That should perform better since it doesn't require shelling out to the mysql client. It also potentially permits programmatic error handling based on the mysql error number if needed.

That said, if you're dynamically generating a large batch of DDL and DML, I could see the approach in this PR being useful. Happy to keep this open and will aim to revisit once open source development is able to resume here, hopefully in a few months.

I've updated the CONTRIBUTING.md to reflect the current situation with PRs. Apologies again for not doing this sooner.