Re: Review Request 69677: RANGER-2287: Improve and optimize db_setup.py file code

2019-01-11 Thread Zsombor Gegesy


> 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

2019-01-11 Thread Zsombor Gegesy (JIRA)


 [ 
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

2019-01-11 Thread Pradeep Agrawal (JIRA)


 [ 
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

2019-01-11 Thread Pradeep Agrawal (JIRA)


[ 
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

2019-01-11 Thread Csaba Koncz (JIRA)


[ 
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

2019-01-11 Thread Csaba Koncz (JIRA)


 [ 
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