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 dbapi2 compatibility driver #145

Merged
merged 10 commits into from
Jul 4, 2024

Conversation

Nicoretti
Copy link
Member

@Nicoretti Nicoretti commented Jul 2, 2024

Migrate dbapi2 compliant shim/driver from SQLA.

@littleK0i
Copy link
Collaborator

littleK0i commented Jul 2, 2024

Not sure if it is a good idea. Historically pyexasol was slightly incompatible with DB API 2.0 on purpose. It forced users to look at documentation for a few minutes instead of mindlessly copying incorrect advice from StackOverflow.

Main issues with DB API 2.0:

  • designed a long time ago;
  • designed for OLTP databases running on a single node;

Fundamentally implementing it would cause more harm than good.

@Nicoretti
Copy link
Member Author

Nicoretti commented Jul 2, 2024

Hi @littleK0i,

I understand and share your concern. That is why I made the shim a separate package. It won't even be part of the pyexasol namespace. If you think the docs may lure some folks into just copying the example without reading it, we could put the part on the alternatives at the end of the document.

Why we are doing this?

We have seen use cases and received requests in the past where DBAPI2 compliance was desired and reasonable, or the only way to integrate with other frameworks. Therefore, we believe we should allow users to achieve this. Although there is turbodbc, it is not a viable alternative in all cases, particularly since it is ODBC-based.

In the end, it is up to the user and their needs. I do not see why we should patronize the library users. We should make the right things easy and the potentially harmful ones harder, but ultimately the user should be able to decide because they should know what they need.

Fundamentally implementing it would do more harm than good.

Having worked intensively with C++ in the past (8+ years) 😬, I totally understand where you're coming from.
However, we strongly believe that this change will be beneficial for some users.

Nevertheless, any suggestions and discussions are welcome so that we can enable the user to make a well-informed decision about the potential tradeoffs at hand.

I hope this helps to better understand the background of this change.
Looking forward to getting further feedback from your side.

best
Nico

🗒️ Side note:
Initially, we considered making it an "undocumented feature". However, finally we where not convinced about this idea because having it undocumented would mean we cannot document the pros and cons (enable the user).

CHANGELOG.md Outdated Show resolved Hide resolved
pyexasol/warnings.py Outdated Show resolved Hide resolved
exasol/driver/websocket/_errors.py Outdated Show resolved Hide resolved
exasol/driver/websocket/_errors.py Outdated Show resolved Hide resolved
Nicoretti and others added 6 commits July 3, 2024 14:20
Co-authored-by: Christoph Kuhnke <github@kuhnke.net>
Co-authored-by: Christoph Kuhnke <github@kuhnke.net>
Co-authored-by: Christoph Kuhnke <github@kuhnke.net>
Co-authored-by: Christoph Kuhnke <github@kuhnke.net>
@Nicoretti Nicoretti requested review from ckunki and tkilias July 3, 2024 12:49
@Nicoretti
Copy link
Member Author

@littleK0i

I'll merge this PR for now. If you have any further feedback or feel that I may have been too quick with this change, please feel free to contact me directly via email. Your feedback is always highly appreciated and welcome.

Best,
Nico

@Nicoretti Nicoretti merged commit 8e3b361 into master Jul 4, 2024
10 checks passed
@Nicoretti Nicoretti deleted the feature/add-dbapi2-compatibility-driver branch July 4, 2024 06:58
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