Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(targets): missing begin()s related to SQLAlchemy 2.0 #1954

Merged
merged 7 commits into from
Sep 13, 2023

Conversation

andyoneal
Copy link
Contributor

@andyoneal andyoneal commented Sep 13, 2023

adds begin() to three other functions where schema is added or columns are altered. because of the changes made to auto-commit in SQLAlchemy 2.0, without begin() the ddl is rolled back.

matches style used in _create_empty_column()


📚 Documentation preview 📚: https://meltano-sdk--1954.org.readthedocs.build/en/1954/

@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Merging #1954 (ee948d3) into main (07198b0) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1954      +/-   ##
==========================================
+ Coverage   87.01%   87.08%   +0.07%     
==========================================
  Files          59       59              
  Lines        5112     5112              
  Branches      827      827              
==========================================
+ Hits         4448     4452       +4     
+ Misses        468      465       -3     
+ Partials      196      195       -1     
Files Changed Coverage Δ
singer_sdk/connectors/sql.py 84.15% <100.00%> (+1.32%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@edgarrmondragon edgarrmondragon requested a review from a team as a code owner September 13, 2023 15:52
Copy link
Collaborator

@edgarrmondragon edgarrmondragon left a comment

Choose a reason for hiding this comment

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

Thanks @andyoneal! This revealed some untested paths so I pushed tests to your branch.

@edgarrmondragon edgarrmondragon merged commit d817e96 into meltano:main Sep 13, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants