Re: Review Request 69677: RANGER-2287: Improve and optimize db_setup.py file code
> 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 > >
[jira] [Resolved] (RANGER-2319) Remove deprecated phantomjs NPM package
[ https://issues.apache.org/jira/browse/RANGER-2319?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Zsombor Gegesy resolved RANGER-2319. Resolution: Fixed Fix Version/s: 2.0.0 Thanks for the patch, applied to [master|https://github.com/apache/ranger/commit/a3394a19f98cea04dc2824fdaa4e3a3fdbd6beb3] > Remove deprecated phantomjs NPM package > --- > > Key: RANGER-2319 > URL: https://issues.apache.org/jira/browse/RANGER-2319 > Project: Ranger > Issue Type: Wish > Components: admin >Affects Versions: 2.0.0 >Reporter: Csaba Koncz >Assignee: Csaba Koncz >Priority: Major > Fix For: 2.0.0 > > > [Phantomjs|https://github.com/apache/ranger/blob/e1b0105eee67bb73c56b66b2dda1c3424555ab3e/security-admin/src/main/webapp/package.json#L15] > NPM package is deprecated. > {code:java} > $ npm show phantomjs > phantomjs@2.1.7 | Apache-2.0 | deps: 8 | versions: 81 > Headless WebKit with JS API > https://github.com/Medium/phantomjs > DEPRECATED ⚠️ - Package renamed to phantomjs-prebuilt. Please update > 'phantomjs' package references to 'phantomjs-prebuilt'{code} > Worse, karma-phantomjs-launcher brings in its version as phantomjs-prebuilt > which results in Phantomjs binaries being downloaded twice. > Build would speed up if only one Phantomjs version is downloaded. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (RANGER-2320) Make db schema patches script idempotent for all DB Flavors
[ https://issues.apache.org/jira/browse/RANGER-2320?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Pradeep Agrawal updated RANGER-2320: Attachment: 0001-RANGER-2320-Make-db-schema-patches-script-idempotent.patch > Make db schema patches script idempotent for all DB Flavors > --- > > Key: RANGER-2320 > URL: https://issues.apache.org/jira/browse/RANGER-2320 > Project: Ranger > Issue Type: Sub-task > Components: Ranger >Affects Versions: 1.0.1, 2.0.0, 1.1.1, 1.2.1 >Reporter: Pradeep Agrawal >Assignee: Pradeep Agrawal >Priority: Major > Fix For: 1.0.1, 2.0.0, 1.1.1, 1.2.1 > > Attachments: > 0001-RANGER-2320-Make-db-schema-patches-script-idempotent.patch > > > RANGER-2291 covers changes only in optimized DB schema script, It seems all > previous DB patches script should be reviewed and if required these script > also should be made idempotent. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (RANGER-2255) Add JavaScript unit tests
[ https://issues.apache.org/jira/browse/RANGER-2255?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16740264#comment-16740264 ] Pradeep Agrawal commented on RANGER-2255: - [~Koncz] It will be okay to make JS Tests run optional. > Add JavaScript unit tests > - > > Key: RANGER-2255 > URL: https://issues.apache.org/jira/browse/RANGER-2255 > Project: Ranger > Issue Type: Wish > Components: admin >Affects Versions: 0.7.0, 2.0.0, 1.2.1 >Reporter: Csaba Koncz >Assignee: Csaba Koncz >Priority: Minor > Fix For: 2.0.0 > > > It would be nice if the admin-ui project would have JavaScript unit tests. > As with RANGER-2220 JavaScript minification was introduced, that can lead to > new type of loading errors that were not seen before. > It would be nice if there was an automatic check that validates the minified > output. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (RANGER-2321) Docker build fails due to PhantomJS dependency
[ https://issues.apache.org/jira/browse/RANGER-2321?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16740238#comment-16740238 ] Csaba Koncz commented on RANGER-2321: - The attached patch builds on RANGER-2319, which has not been merged yet. It implements two changes: # build_ranger_using_docker.sh adds bzip2 and fontconfig to the image being built, so the build succeeds with the new image. # JavaScript test execution can be switched of by passing -DskipJSTests to Maven. This can be used when building outside Docker > Docker build fails due to PhantomJS dependency > -- > > Key: RANGER-2321 > URL: https://issues.apache.org/jira/browse/RANGER-2321 > Project: Ranger > Issue Type: Bug > Components: admin >Affects Versions: 2.0.0 >Reporter: Csaba Koncz >Assignee: Csaba Koncz >Priority: Major > Attachments: > 0001-RANGER-2321-Docker-build-fails-due-to-PhantomJS-depe.patch > > > Docker build fails in an early phase do to the PhantomJS dependency > introduced in RANGER-2255. E.g. running > {code:java} > ./build_ranger_using_docker.sh mvn clean verify -am -pl security-admin{code} > results in > {code:java} > [INFO] < org.apache.ranger:security-admin-web > > > [INFO] Building Security Admin Web Application 2.0.0-SNAPSHOT [7/7] > [INFO] [ war > ]- > [INFO] > [INFO] --- maven-clean-plugin:2.6.1:clean (default-clean) @ > security-admin-web --- > [INFO] > [INFO] --- maven-enforcer-plugin:1.4.1:enforce (enforce-maven-version) @ > security-admin-web --- > [INFO] > [INFO] --- maven-enforcer-plugin:1.4.1:enforce (enforce-versions) @ > security-admin-web --- > [INFO] > [INFO] --- maven-resources-plugin:2.7:copy-resources (copy-resources) @ > security-admin-web --- > [INFO] Using 'UTF-8' encoding to copy filtered resources. > [INFO] Copying 2 resources > [INFO] > [INFO] --- maven-remote-resources-plugin:1.5:process > (process-resource-bundles) @ security-admin-web --- > [INFO] > [INFO] --- maven-resources-plugin:2.7:resources (default-resources) @ > security-admin-web --- > [INFO] Using 'UTF-8' encoding to copy filtered resources. > [INFO] Copying 22 resources > [INFO] Copying 3 resources > [INFO] > [INFO] --- maven-antrun-plugin:1.7:run (default) @ security-admin-web --- > [INFO] Executing tasks > main: > [INFO] Executed tasks > [INFO] > [INFO] --- frontend-maven-plugin:1.6:install-node-and-npm (install node and > npm) @ security-admin-web --- > [INFO] Installing node version v8.12.0 > [INFO] Unpacking > /home/builder/.m2/repository/com/github/eirslett/node/8.12.0/node-8.12.0-linux-x64.tar.gz > into /ranger/security-admin/target/node/tmp > [INFO] Copying node binary from > /ranger/security-admin/target/node/tmp/node-v8.12.0-linux-x64/bin/node to > /ranger/security-admin/target/node/node > [INFO] Installed node locally. > [INFO] Installing npm version 6.4.1 > [INFO] Unpacking > /home/builder/.m2/repository/com/github/eirslett/npm/6.4.1/npm-6.4.1.tar.gz > into /ranger/security-admin/target/node/node_modules > [INFO] Installed npm locally. > [INFO] > [INFO] --- frontend-maven-plugin:1.6:npm (npm install) @ security-admin-web > --- > [INFO] Running 'npm install' in /ranger/security-admin/target > [INFO] > [INFO] > phantomjs-prebuilt@2.1.16 install > /ranger/security-admin/target/node_modules/karma-phantomjs-launcher/node_modules/phantomjs-prebuilt > [INFO] > node install.js > [INFO] > [INFO] PhantomJS not found on PATH > [INFO] Downloading > https://github.com/Medium/phantomjs/releases/download/v2.1.1/phantomjs-2.1.1-linux-x86_64.tar.bz2 > [INFO] Saving to /tmp/phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2 > [INFO] Receiving... > [INFO] > [INFO] Received 22866K total. > [INFO] Extracting tar contents (via spawned process) > [ERROR] Error extracting archive > [ERROR] Phantom installation failed { Error: Command failed: tar jxf > /tmp/phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2 > [ERROR] tar (child): bzip2: Cannot exec: No such file or directory > [ERROR] tar (child): Error is not recoverable: exiting now > [ERROR] tar: Child returned status 2 > [ERROR] tar: Error is not recoverable: exiting now > [ERROR] > [ERROR] at ChildProcess.exithandler (child_process.js:276:12) > [ERROR] at emitTwo (events.js:126:13) > [ERROR] at ChildProcess.emit (events.js:214:7) > [ERROR] at maybeClose (internal/child_process.js:915:16) > [ERROR] at Socket.stream.socket.on (internal/child_process.js:336:11) > [ERROR] at emitOne (events.js:116:13) > [ERROR] at Socket.emit (events.js:211:7) > [ERROR] at Pipe._handle.close [as _onclose] (net.js:561:12) > [ERROR] killed: false, > [ERROR] code: 2, > [ERROR] signal: null, > [ERROR] cmd: 'tar jxf /tmp/phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2' } > Error:
[jira] [Updated] (RANGER-2321) Docker build fails due to PhantomJS dependency
[ https://issues.apache.org/jira/browse/RANGER-2321?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Csaba Koncz updated RANGER-2321: Attachment: 0001-RANGER-2321-Docker-build-fails-due-to-PhantomJS-depe.patch > Docker build fails due to PhantomJS dependency > -- > > Key: RANGER-2321 > URL: https://issues.apache.org/jira/browse/RANGER-2321 > Project: Ranger > Issue Type: Bug > Components: admin >Affects Versions: 2.0.0 >Reporter: Csaba Koncz >Assignee: Csaba Koncz >Priority: Major > Attachments: > 0001-RANGER-2321-Docker-build-fails-due-to-PhantomJS-depe.patch > > > Docker build fails in an early phase do to the PhantomJS dependency > introduced in RANGER-2255. E.g. running > {code:java} > ./build_ranger_using_docker.sh mvn clean verify -am -pl security-admin{code} > results in > {code:java} > [INFO] < org.apache.ranger:security-admin-web > > > [INFO] Building Security Admin Web Application 2.0.0-SNAPSHOT [7/7] > [INFO] [ war > ]- > [INFO] > [INFO] --- maven-clean-plugin:2.6.1:clean (default-clean) @ > security-admin-web --- > [INFO] > [INFO] --- maven-enforcer-plugin:1.4.1:enforce (enforce-maven-version) @ > security-admin-web --- > [INFO] > [INFO] --- maven-enforcer-plugin:1.4.1:enforce (enforce-versions) @ > security-admin-web --- > [INFO] > [INFO] --- maven-resources-plugin:2.7:copy-resources (copy-resources) @ > security-admin-web --- > [INFO] Using 'UTF-8' encoding to copy filtered resources. > [INFO] Copying 2 resources > [INFO] > [INFO] --- maven-remote-resources-plugin:1.5:process > (process-resource-bundles) @ security-admin-web --- > [INFO] > [INFO] --- maven-resources-plugin:2.7:resources (default-resources) @ > security-admin-web --- > [INFO] Using 'UTF-8' encoding to copy filtered resources. > [INFO] Copying 22 resources > [INFO] Copying 3 resources > [INFO] > [INFO] --- maven-antrun-plugin:1.7:run (default) @ security-admin-web --- > [INFO] Executing tasks > main: > [INFO] Executed tasks > [INFO] > [INFO] --- frontend-maven-plugin:1.6:install-node-and-npm (install node and > npm) @ security-admin-web --- > [INFO] Installing node version v8.12.0 > [INFO] Unpacking > /home/builder/.m2/repository/com/github/eirslett/node/8.12.0/node-8.12.0-linux-x64.tar.gz > into /ranger/security-admin/target/node/tmp > [INFO] Copying node binary from > /ranger/security-admin/target/node/tmp/node-v8.12.0-linux-x64/bin/node to > /ranger/security-admin/target/node/node > [INFO] Installed node locally. > [INFO] Installing npm version 6.4.1 > [INFO] Unpacking > /home/builder/.m2/repository/com/github/eirslett/npm/6.4.1/npm-6.4.1.tar.gz > into /ranger/security-admin/target/node/node_modules > [INFO] Installed npm locally. > [INFO] > [INFO] --- frontend-maven-plugin:1.6:npm (npm install) @ security-admin-web > --- > [INFO] Running 'npm install' in /ranger/security-admin/target > [INFO] > [INFO] > phantomjs-prebuilt@2.1.16 install > /ranger/security-admin/target/node_modules/karma-phantomjs-launcher/node_modules/phantomjs-prebuilt > [INFO] > node install.js > [INFO] > [INFO] PhantomJS not found on PATH > [INFO] Downloading > https://github.com/Medium/phantomjs/releases/download/v2.1.1/phantomjs-2.1.1-linux-x86_64.tar.bz2 > [INFO] Saving to /tmp/phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2 > [INFO] Receiving... > [INFO] > [INFO] Received 22866K total. > [INFO] Extracting tar contents (via spawned process) > [ERROR] Error extracting archive > [ERROR] Phantom installation failed { Error: Command failed: tar jxf > /tmp/phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2 > [ERROR] tar (child): bzip2: Cannot exec: No such file or directory > [ERROR] tar (child): Error is not recoverable: exiting now > [ERROR] tar: Child returned status 2 > [ERROR] tar: Error is not recoverable: exiting now > [ERROR] > [ERROR] at ChildProcess.exithandler (child_process.js:276:12) > [ERROR] at emitTwo (events.js:126:13) > [ERROR] at ChildProcess.emit (events.js:214:7) > [ERROR] at maybeClose (internal/child_process.js:915:16) > [ERROR] at Socket.stream.socket.on (internal/child_process.js:336:11) > [ERROR] at emitOne (events.js:116:13) > [ERROR] at Socket.emit (events.js:211:7) > [ERROR] at Pipe._handle.close [as _onclose] (net.js:561:12) > [ERROR] killed: false, > [ERROR] code: 2, > [ERROR] signal: null, > [ERROR] cmd: 'tar jxf /tmp/phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2' } > Error: Command failed: tar jxf > /tmp/phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2 > [ERROR] tar (child): bzip2: Cannot exec: No such file or directory > [ERROR] tar (child): Error is not recoverable: exiting now > [ERROR] tar: Child returned status 2 > [ERROR] tar: Error is not recoverable: exiting now > [ERROR] > [ERROR] at