-
Notifications
You must be signed in to change notification settings - Fork 69
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
feat: SQLTarget connector instance shared with sinks #1864
feat: SQLTarget connector instance shared with sinks #1864
Conversation
@BuzzCutNorman can you try fixing the mypy failure? Otherwise I can take look in the coming days. |
…ks' of https://github.com/BuzzCutNorman/sdk into 1772-sqltarget-level-connector-instance-shared-with-sinks
Codecov Report
@@ Coverage Diff @@
## main #1864 +/- ##
==========================================
+ Coverage 86.87% 86.89% +0.02%
==========================================
Files 59 59
Lines 5027 5060 +33
Branches 814 819 +5
==========================================
+ Hits 4367 4397 +30
- Misses 464 466 +2
- Partials 196 197 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@edgarrmondragon I was able to get the mypy failures resolved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @BuzzCutNorman, overall this is looking great. I still have to go through the API and see how this fits with our plans for connectors (at first glance it seems it does!).
Co-authored-by: Edgar R. M. <edgarrm358@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't look like we need to update the cookiecutter templates, can you confirm that?
You are correct the cookiecutter templates do not need updating. In my initial version they did need to be updated. Then thanks to the sdk test I realized there was a way to accomplish this without adding a |
Thanks for confirming! |
Thanks @BuzzCutNorman! |
This adds a variable
_target_connector
toSQLTarget
to house an instance of thedefault_sink_class.connector_class
and a property to access it. The methodget_sink
was brought down fromTarget
and altered to calladd_sqlsink
. The methodadd_sqlsink
is a copy of theadd_sink
method inTarget
it just passes the shared connector instance to theSQLSink
isntances it initalizes for each stream.need some assistance with making this mypy compliant.
Closes #1772
📚 Documentation preview 📚: https://meltano-sdk--1864.org.readthedocs.build/en/1864/