getindata / dbt-flink-adapter

Adapter for dbt that executes dbt pipelines on Apache Flink
Apache License 2.0
80 stars 10 forks source link

mock flink sql gateway client #59

Closed zqWu closed 1 year ago

zqWu commented 1 year ago

Description

a mock sql flink client Resolves https://github.com/getindata/dbt-flink-adapter/issues/34

PR Checklist
gliter commented 1 year ago

I hope you don't mind. I have converted this into draft. I see that you are still working on it. Please use Ready for review button once this is ready.

zqWu commented 1 year ago

test_seeds.py passed but other tests, fail becase they need get result from "client", which hasn't implemented

zqWu commented 1 year ago

done

gliter commented 1 year ago

@zqWu Sorry for the delay. Had finally a chance to actually run properly your PR. This looks really great! Thank you for that work. I have left some comments. Looked like you initially wanted to substitute flink.sqlgateway.client.FlinkSqlGatewayClient and later changed your mind to use http mock. Am I right? There are leftovers from your initial idea.

Thanks for changing it to use instance. I am not familiar with that library, would it work if we would create few instances (but using different port each time) in parallel? This is problem for another time anyway.

Also in the future I guess we will need to add more complex behaviors like making it return error or some specific response.

Please check my comments and then I can merge it.

zqWu commented 1 year ago

thanks for review, i'll handle the problems above

zqWu commented 1 year ago

i have implemented a mock sqlgateway, with some test code. still in progress

i think, it will be useful for some test ( not stream and some flink specific sql)

for real e2e test when not use default_catalog, it requires:

these works seems heavy to repeat with mock gateway can sav a lot effort it can run in debug mode in pycharm, pretty friend for beginners like me

gliter commented 1 year ago

Should be doable using testcontainers. This is something to be left for the future.

gliter commented 1 year ago

The file paths are unnecessary nested. Component tests should just be in tests/component instead of test/component/mock_gw/tests.

zqWu commented 1 year ago

my branch for this pr is messy, gonna close this and promot another