draft

[michaelkerry]

The idea here is to provide a pluggable collection of utils to enable easy interactions with databases of various types.

I've started with oracle db bc the example project into which this package is integrated (a flask api example project) uses an oracle db, but the intent is to enable additional modules for mysql, postgres, etc.

I've updated the documentation in a way that I think makes sense - I was able to start a python console session and follow those instructions (with valid connection information) and pull back results. Please let me know what else might be helpful. One section to be added at beginning: 'Purpose and Intended Audience'

[jazahn]

Wrap code examples in ticks.

I'd also like to see a super streamlined example of it put together like:

from pydb import OracleDb
from pylog import logger

db = OracleDb(host="valid_host", port=8003, service="SERVICE_NAME", user="username", pwd="pwd")
result = db.execute_query("select * from emp")
logger.info(json.dumps(result))

db.cleanup()

[michaelkerry]

code wrapped; integrated example added

[ccurreri]

See the individual comments. Some small design things that seems strange to me, but might make sense in your context. I only commented on the first instances that I saw but some of them are done multiple times.

Chiefly, you have a helper method that gets the connection pool but also expose it as a public attribute. You may want to consider refactoring to make it more clear that there is only one way that folks should be interacting with the pool object.

[ccurreri]

Is there a specific reason you're overriding this?

You already are handling this with the abstractmethod decorators. If a subclass fails to implement one of these it can't be instantiated.

>>> from abc import ABC, abstractmethod
>>>
>>> class Foo(ABC):
...     @abstractmethod
...     def bar(self):
...             pass
...
>>> class Baz(Foo):
...     pass
...
>>> x = Baz()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Can't instantiate abstract class Baz with abstract method bar

[michaelkerry]

updated - removed

[ccurreri]

The else clause here technically will never fire because you always set self.pool in __init__ or throw an error.

One alternative would be to just use a property for this to load it on demand:

class OracleDB(DBInterface):
    def __init__(self, host: str, port: int, service: str, user: str, pwd: str,
                       logging_level: int = 50, logging_format: logging.Formatter = None):

        . . .

        self._pool = None

        . . .
        
    @property
    def pool(self):
        if not self._pool:
           self._pool = self.set_up_session_pool()

        return self._pool

[michaelkerry]

updated

[ccurreri]

Could just be:

return self.get_session_pool().acquire()

or with the changes above just:

return self.pool.acquire()

[michaelkerry]

updated

[ccurreri]

Do you mean return create_row() here or do you actually mean to return the function?

If the latter this is something a lambda could do relatively easily:

return lambda *args: dict(zip(column_names, args))

[michaelkerry]

I 'inherited' this syntax - and am opting to leave as is since it is working as expected