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

Add native IPv4 and IPv6 types support #52

Merged
merged 1 commit into from
Apr 9, 2019
Merged

Add native IPv4 and IPv6 types support #52

merged 1 commit into from
Apr 9, 2019

Conversation

AchilleAsh
Copy link
Contributor

This PR adds IPv4 and IPv6 support in sqlalchemy, following the work done in clickhouse-driver.

Note that as no release of clickhouse-driver with IP support is available in pip yet, travis will fail for now. Nonetheless all tests are passing on my computer on python 3.6 and 2.7 (with clickhouse 19.3.3+).

I have also added the require_server_version decorator to be used in tests so IP related tests run only on 19.3.3+

Finally as clickhouse only support basic comparators for IP, I have added the in and not in operators to generate proper queries, see tests for examples.

@coveralls
Copy link

coveralls commented Mar 3, 2019

Coverage Status

Coverage increased (+0.8%) to 91.264% when pulling 53bdbb6 on AchilleAsh:master into 139ae52 on xzkostyan:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.4%) to 89.572% when pulling da040bf on AchilleAsh:master into 91453b1 on xzkostyan:master.

@xzkostyan
Copy link
Owner

Great. I'll review PR in a couple of weeks. I'm on vacation right now.

Copy link
Owner

@xzkostyan xzkostyan left a comment

Choose a reason for hiding this comment

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

Everything else looks good for me.


def bind_processor(self, dialect):
def process(value):
return unicode(value)
Copy link
Owner

Choose a reason for hiding this comment

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

There is compatible helper for this.

from .util import compat
return compat.text_type(text)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops didn't see it, fixed!

setup.py Outdated
install_requires = [
'sqlalchemy>=1.2',
'requests',
'clickhouse_driver>=0.0.14'
Copy link
Owner

Choose a reason for hiding this comment

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

I've bumped clickhouse-driver version to 0.0.19 for ip addresses support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Version bumped

@xzkostyan xzkostyan merged commit 7e4df65 into xzkostyan:master Apr 9, 2019
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.

3 participants