> On Jan. 8, 2019, 11:48 a.m., Zsombor Gegesy wrote:
> > It's great news, that you could delete thousands of lines of repetitive
> > code, however you could achieve more, if instead of putting everything into
> > one class, and put
> > '''
> > if self.XA_DB_FLAVOR == DB_MYSQL:
> > ...
> > elif self.XA_DB_FLAVOR == DB_POSTGRES:
> > ...
> > '''
> >
> > You can write
> > self.do_something(...)
> >
> > and implement do_something differently in the MySQL/PostgreSQL/Oracle
> > specific adapter class
>
> Pradeep Agrawal wrote:
> There shall be too many self.do_something(...) function I have to write
> which shall look like the previous code. Can you review it once again and let
> me know with few examples.
Maybe you can add:
'''
def execute_query(self, query):
''' Execute query and return the output as a string '''
get_cmd = self.get_jisql_cmd(self.db_user, self.db_password, self.db_name)
if is_unix:
full_command = get_cmd + " -query \"" + query + "\""
elif os_name == "WINDOWS":
full_command = get_cmd + " -query \"" + query + "\" -c ;"
else:
raise Exception("This OS is not supported!")
jisql_log(full_command, self.db_password)
output = check_output(query)
return output
def execute_update(self, update):
''' Execute the update query and return the error code'''
get_cmd = self.get_jisql_cmd(self.db_user, self.db_password, self.db_name)
if is_unix:
full_command = get_cmd + " -query \"" + update + "\""
jisql_log(full_command, self.db_password)
return subprocess.call(shlex.split(query))
elif os_name == "WINDOWS":
full_command = get_cmd + " -query \"" + update + "\" -c ;"
jisql_log(full_command, self.db_password)
ret = subprocess.call(query)
raise Exception("This OS is not supported!")
'''
So you can get rid of lot's of repeating code around to support Windows.
And for the db changes, I would imagine something like this:
'''
class BaseDB(object):
@abstractmethod
def get_stale_patch_query(self, version, client_host,
stalePatchEntryHoldTimeInMinutes):
pass
class MysqlConf(BaseDB):
def get_stale_patch_query(self, version, client_host,
stalePatchEntryHoldTimeInMinutes):
return "select version from x_db_version_h where version = '%s' and
active = 'N' and updated_by='%s' and
TIMESTAMPDIFF(MINUTE,inst_at,CURRENT_TIMESTAMP)>=%s;" % (version, client_host,
stalePatchEntryHoldTimeInMinutes)
'''
So you can write:
'''
output =
self.execute_query(self.get_stale_patch_query(version,client_host,stalePatchEntryHoldTimeInMinutes))
...
'''
What do you think, does it makes sense?
- Zsombor
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69677/#review211760
-----------------------------------------------------------
On Jan. 7, 2019, 6:37 a.m., Pradeep Agrawal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69677/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2019, 6:37 a.m.)
>
>
> Review request for ranger, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh,
> Nikhil P, Ramesh Mani, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-2287
> https://issues.apache.org/jira/browse/RANGER-2287
>
>
> Repository: ranger
>
>
> Description
> -------
>
> **Problem Statement:** There are lot of repeated code in db_setup.py which
> can be removed which shall help developers to make any changes in db_setup.py
> in future.
>
> **Proposed Solution:** Proposed patch shall remove the db setup methods of
> each db flavor and shall use a single method for a specific work for each db
> flavor. Based on the db flavor, config values shall be populated and handled
> in the code after this patch.
>
>
> Diffs
> -----
>
> security-admin/scripts/db_setup.py f1223b38c
>
>
> Diff: https://reviews.apache.org/r/69677/diff/1/
>
>
> Testing
> -------
>
> **Use Cases covered for all the db flavors:**
> *1. Fresh installation(Ranger 2.0):* Tested patch with fresh installation of
> ranger admin.
> *2. Upgrade(from 0.7 to 2.0):* Installed Ranger from 0.7 branch and used same
> db config on Ranger 2.0 installation config and run the setup.sh. Ranger was
> upgraded successfully.
>
>
> Thanks,
>
> Pradeep Agrawal
>
>
