-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feature db2 transport #6
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few REST/HTTP residual references and some notes
|
||
class Db2Connection: | ||
""" | ||
Represents a REST HTTP connection that can be used to get the Python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
REST HTTP -> Database
@@ -0,0 +1,61 @@ | |||
import requests | |||
import json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused imports
@@ -0,0 +1,24 @@ | |||
import unittest | |||
import os |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import
def __init__(self, database, username, password): | ||
|
||
self.connection = dbi.connect(database=database, \ | ||
user=username, password=password) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible to just wrap ibm_db_dbi.connect
:
def __init__(self, *args, **kwargs):
self.connection = dbi.connect(args, kwargs)
https://www.digitalocean.com/community/tutorials/how-to-use-args-and-kwargs-in-python-3
This would allow users to pass any connection options they want, though is that something we actually want them to be able to do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, probably. Thoughts on having db, username, and password explicit (for comprehension), and adding **kwargs on the end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you can have as many named parameters as you like followed by *args
and **kwargs
, however this would mess up positional argument handling unless you had the same named arguments and in the same order as ibm_db_dbi.connect
. So I'd suggest only supporting kwargs
.
Does that make sense?
""" | ||
if not response.ok: | ||
raise TransportError("There was an error while executing the payload.") | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Residual REST code
|
||
def __test_connection(self): | ||
""" | ||
Test that the DB2Sock REST transport exists and works properly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/REST/DB2/g
{'s': {'name': 'char', 'type': '128a', 'value': 'Hi there'}} | ||
] | ||
}) | ||
response = toolkit.execute() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No assertion? Looks like the test would never fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had a _test_connection funciton in the HTTP transport, because the HTTP transport didn
t actually "connect". Am removing the _test_connection function here.
Great enhancement! Key to the success of this project. We definitely want |
added db2 transport with ibm_db_dbi