refactor: make DDL overridable for column ADD, ALTER, and RENAME operations
#1114
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1114 +/- ##
==========================================
- Coverage 83.52% 83.52% -0.01%
==========================================
Files 42 42
Lines 3860 3872 +12
Branches 657 657
==========================================
+ Hits 3224 3234 +10
- Misses 472 474 +2
Partials 164 164
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
kgpayne
requested review from
edgarrmondragon,
cjohnhanson and
aaronsteers
as code owners
|
Codecov is down by 0.52% ⬇️ As this is a refactor, and as I have a handful of other things for tap/target snowflake to get through to get to done, I'd prefer not to add testing on the refactored methods unless anyone feels very strongly about them 🙂 |
kgpayne
changed the title
aaronsteers
changed the title
ADD, ALTER, and RENAME operations
aaronsteers
changed the title
ADD, ALTER, and RENAME operationsADD, ALTER, and RENAME operations
@kgpayne - Do you mind adding a simple test for the DDL static method evaluation? Specifically, I'm thinking of a test for the static methods which are easily tested from a class reference and don't need the a full sink object to be created. If they return the DDL or string result expected when passed specific input values, I think that's probably reasonable. |
|
Renamed column add (previously create) and added tests ✅ Codecov still isn't happy though 🤦♂️ |
There was a problem hiding this comment.
lgtm, the SQL API is coming together! @kgpayne the dependency review error is a false positive afaict, so feel free to merge 😄
* Closes #26 * Closes #18 * Closes #16 - completes the TODO related to meltano/sdk#1114 - Adds a default batch config because its required. Default to writing locally. - Add copy syntax if key properties dont exist. Previously the merge logic was invalid due to syntax errors. Apparently my first test data didnt have key properties - `conform_record_data_types` requires a new `level` kwarg and was failing because it was missing. - the `column_exists` methods was being called a ton and was querying snowflake for all columns every time which was slow so I have it caching now - the `schema_exists` method failed for me because `get_schema_names` returns all lower columns. It errored because it was trying to create a schema that already existed. - bumped SDK to 0.27.0 - after bumping the SDK I needed to address #16 which means `create_sqlalchemy_engine` was effectively replaced with `create_engine` - Also fix import failure of `lazy_chunked_generator` that was moved in SDK I'm able to test that this works with the sample tap-github data from the getting started guide. --------- Co-authored-by: Edgar R. M. <edgarrm358@gmail.com>
Closes #1033
ALTER TABLEandALTER COLUMNsyntax to be overridden #1033📚 Documentation preview 📚: https://meltano-sdk--1114.org.readthedocs.build/en/1114/