Skip to content

Conversation

@fsiler
Copy link

@fsiler fsiler commented Sep 7, 2017

I don't know what it is about pymssql- had enough trouble getting it working on py3, and then it wouldn't cooperate on py2. I therefore used 2to3 and it didn't horribly mangle anything. I've also added some documentation and thinking, though it probably could use better documentation. There's much more to do, including proper dependencies, but it's a start.

@fsiler fsiler closed this Sep 7, 2017
@fsiler fsiler reopened this Sep 7, 2017
@fsiler
Copy link
Author

fsiler commented Sep 7, 2017

so, tox was wigging out over a comment in MSSQL connector file, and now I'm not sure what it dislikes- it may be the fact that my warning will kill py2 when you try to use the MSSQL connector, and this is by design.

@DonDebonair
Copy link
Member

DonDebonair commented Sep 8, 2017

Great that you're working on this!

Looking at your code, can I conclude that not only do we get a MSSQL plugin, we also get Python 3 support to boot? That'd be awesome.

As a sidenote question: why do you require Python 3.6-dev when testing with Tox?

edit: I prefer absolute imports over relative nowadays. This is something my colleague and I did wrong when we originally wrote Cornet. Absolute imports seem to be the least ambiguous option, working for both Python 2 and 3.

So instead of:

from .mysql_connector import MySqlConnector

We should do:

from cornet.connectors.mysql_connector import MySqlConnector

Thoughts?

@mkrcah
Copy link
Member

mkrcah commented Sep 14, 2017

@fsiler Are you still working on this?

@fsiler
Copy link
Author

fsiler commented Sep 24, 2017

yes, sorry, still alive- I need to dig deeper into the CI and figure out how to get better and working tests, but happy to fix the imports.

@brunowego
Copy link

Great job here @fsiler, can you fix the issues?

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.

4 participants