Skip to content
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

fix raw connection execute #40

Merged
merged 2 commits into from
Nov 30, 2018
Merged

fix raw connection execute #40

merged 2 commits into from
Nov 30, 2018

Conversation

AchilleAsh
Copy link
Contributor

Following my bug report #39 , here's a simple check on context during execute preparation.

It does fix the raw connection execution and make no changes when a context is set (vast majority of use cases) however it implies that no execution options can be passed when in raw mode.

@coveralls
Copy link

coveralls commented Nov 29, 2018

Coverage Status

Coverage increased (+0.03%) to 88.587% when pulling 97ef5b7 on AchilleAsh:master into b77fc74 on xzkostyan:master.

@xzkostyan
Copy link
Owner

Hi.

This looks fine. But, please add simple test to cover this case:

class SomeTestCase(NativeSessionTestCase):
    def test_example(self):
        raw = self.session.bind.raw_connection()
        cur = raw.cursor()

        cur.execute("SELECT * FROM system.numbers LIMIT 10")
        # assert smth

@AchilleAsh
Copy link
Contributor Author

Hi,

I've added 2 tests (one with context, the other without) so both conditions in _prepare are tested.

@xzkostyan xzkostyan merged commit 612fdd4 into xzkostyan:master Nov 30, 2018
@Matt-Texier
Copy link

Hi Guys, sorry to bother you with this but any plans to tag a new release that include this fix ?

@xzkostyan
Copy link
Owner

@Matt-Texier no problem. I've just published 0.0.9 version with this fix.

@Matt-Texier
Copy link

@xzkostyan many thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants