[Asterisk-code-review] Support testing realtime with ODBC (testsuite[master])

[Mark Michelson]

Mark Michelson has posted comments on this change.

Change subject: Support testing realtime with ODBC
......................................................................


Patch Set 1:

(4 comments)

I'm not -1-ing this mainly because my findings are just questions rather than issues. I think there are parts of this that just aren't clicking with me.

https://gerrit.asterisk.org/#/c/3026/1/lib/python/asterisk/realtime_odbc_module.py
File lib/python/asterisk/realtime_odbc_module.py:

PS1, Line 70:        # inform ODBC where to find them
            :         os.environ['ODBCSYSINI'] = etc
I'm not very familiar with the requirements of ODBC, but is changing the environment variable enough to be sure that ODBC will use the proper configs? Is it not also a requirement to restart the unixODBC service?

Also, does this change to the environment variable linger after the test completes? Does this need to be reset when a test completes?


PS1, Line 76:        inst = self._read_ini_file('/etc/odbcinst.ini')
            :         found_driver = None
            :         for section in inst:
            :             driver = inst[section].get('Driver')
            :             if driver and match_driver in driver:
            :                 found_driver = section
            :                 break
Can you explain what the purpose of this block is? It looks like you're trying to verify that the driver specified in the configuration is also in some section of the /etc/odbcinst.ini file. However, I don't understand what this accomplishes. Wouldn't it be more useful to ensure that the driver exists in the file system?

It also seems like you're prohibited from providing a 100% original odbcinst configuration here. You have to have an existing /etc/odbcinst.ini. Why is that?


PS1, Line 85:             LOGGER.error('Unable to find odbcinst entry matching driver ' +
            :                          repr(driver))
Did you mean to have match_driver here instead of driver?


PS1, Line 101:        # update the Database path if relative
             :         Database = self.odbc[dsn].get('Database', None)
             :         if Database and Database[0] != '/' and '/' in Database:
             :             self.odbc[dsn]['Database'] = '/'.join([
             :                 self.test_object.ast[0].base,
             :                 Database])
I don't really understand what this is doing. If I have Postgres set up and have created a database called "asterisk" in it that has the configuration needed for a test, then this will come along and instead set the odbc.ini file to say to use a different database name instead. Therefore, I don't think the test would end up finding the database I expect it to.


-- 
To view, visit https://gerrit.asterisk.org/3026
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e20dc7354e0d4bc7d1b5d5458bdd356f1b00d26
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Scott Griepentrog <sgriepentrog at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-HasComments: Yes