You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by shivzone <gi...@git.apache.org> on 2017/10/12 18:40:21 UTC
[GitHub] incubator-hawq pull request #1301: Re-added pxf profile default to rpm and t...
GitHub user shivzone opened a pull request:
https://github.com/apache/incubator-hawq/pull/1301
Re-added pxf profile default to rpm and tar tasks
The tar and the rpm targets were missing pxf-profiles-default.xml.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shivzone/incubator-hawq HAWQ-1538
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/1301.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1301
----
commit c1f4b29adc0f4bcc9bb77dfe65d046c8c8f41517
Author: shivzone <sh...@gmail.com>
Date: 2017-10-12T18:39:20Z
Re-added pxf profile default to rpm and tar tasks
----
---
[GitHub] incubator-hawq pull request #1301: Re-added pxf profile default to rpm and t...
Posted by lavjain <gi...@git.apache.org>.
Github user lavjain commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1301#discussion_r144380841
--- Diff: pxf/build.gradle ---
@@ -310,6 +315,7 @@ project('pxf-service') {
project.distTar {
from('src/main/resources/pxf-profiles.xml') { into 'conf' }
+ from('src/main/resources/pxf-profiles-default.xml') { into 'conf' }
--- End diff --
This may not work because pxf-profiles-default.xml has to be copied to pxf-profiles.xml
---
[GitHub] incubator-hawq issue #1301: Re-added pxf profile default to rpm and tar task...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on the issue:
https://github.com/apache/incubator-hawq/pull/1301
@lavjain pxf-profiles.xml is better to be empty to prevent users from incorrectly updating the default entires in it. This is similar to the separation we have between pxf-private.classpath from pxf-public.classpath.
@lavjain @sansanichfb does that mean we don't need pxf-profiles.default ?
---
[GitHub] incubator-hawq pull request #1301: Re-added pxf profile default to rpm and t...
Posted by lavjain <gi...@git.apache.org>.
Github user lavjain commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1301#discussion_r144388153
--- Diff: pxf/build.gradle ---
@@ -310,6 +315,7 @@ project('pxf-service') {
project.distTar {
from('src/main/resources/pxf-profiles.xml') { into 'conf' }
+ from('src/main/resources/pxf-profiles-default.xml') { into 'conf' }
--- End diff --
@sansanichfb just verified in the code that we are loading both profiles.
---
[GitHub] incubator-hawq pull request #1301: Re-added pxf profile default to rpm and t...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1301#discussion_r144450132
--- Diff: pxf/build.gradle ---
@@ -310,6 +315,7 @@ project('pxf-service') {
project.distTar {
from('src/main/resources/pxf-profiles.xml') { into 'conf' }
+ from('src/main/resources/pxf-profiles-default.xml') { into 'conf' }
--- End diff --
@lavjain pxf-profiles.xml is better to be empty to prevent users from incorrectly updating the default entires in it. This is similar to the separation we have between pxf-private.classpath from pxf-public.classpath.
@lavjain @sansanichfb does that mean we don't need pxf-profiles.default ?
---
[GitHub] incubator-hawq pull request #1301: Re-added pxf profile default to rpm and t...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone closed the pull request at:
https://github.com/apache/incubator-hawq/pull/1301
---
[GitHub] incubator-hawq pull request #1301: Re-added pxf profile default to rpm and t...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1301#discussion_r144382134
--- Diff: pxf/build.gradle ---
@@ -310,6 +315,7 @@ project('pxf-service') {
project.distTar {
from('src/main/resources/pxf-profiles.xml') { into 'conf' }
+ from('src/main/resources/pxf-profiles-default.xml') { into 'conf' }
--- End diff --
For the product's perspective why is that needed ?
---
[GitHub] incubator-hawq issue #1301: Re-added pxf profile default to rpm and tar task...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on the issue:
https://github.com/apache/incubator-hawq/pull/1301
After some investigation, the webapp does have pxf-profiles-default.xml file in pxf-service/webapps/pxf/WEB-INF/classes/ along with other configuration files. @lavjain is right, we dont' need pxf-profiles-default.xml in conf/.
---
[GitHub] incubator-hawq pull request #1301: Re-added pxf profile default to rpm and t...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1301#discussion_r144383133
--- Diff: pxf/build.gradle ---
@@ -310,6 +315,7 @@ project('pxf-service') {
project.distTar {
from('src/main/resources/pxf-profiles.xml') { into 'conf' }
+ from('src/main/resources/pxf-profiles-default.xml') { into 'conf' }
--- End diff --
Not really. The purpose of pxf-profile-default.xml is to have all the default profiles. The users need to only add custom profiels to pxf-profile.xml. The PXF application loads both the xml files during startup.
---
[GitHub] incubator-hawq pull request #1301: Re-added pxf profile default to rpm and t...
Posted by lavjain <gi...@git.apache.org>.
Github user lavjain commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1301#discussion_r144382742
--- Diff: pxf/build.gradle ---
@@ -310,6 +315,7 @@ project('pxf-service') {
project.distTar {
from('src/main/resources/pxf-profiles.xml') { into 'conf' }
+ from('src/main/resources/pxf-profiles-default.xml') { into 'conf' }
--- End diff --
So, people would have to manually copy it then?
---
[GitHub] incubator-hawq pull request #1301: Re-added pxf profile default to rpm and t...
Posted by lavjain <gi...@git.apache.org>.
Github user lavjain commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1301#discussion_r144383457
--- Diff: pxf/build.gradle ---
@@ -310,6 +315,7 @@ project('pxf-service') {
project.distTar {
from('src/main/resources/pxf-profiles.xml') { into 'conf' }
+ from('src/main/resources/pxf-profiles-default.xml') { into 'conf' }
--- End diff --
Then why did it fail for pxf-automation on HAWQ?
---
[GitHub] incubator-hawq pull request #1301: Re-added pxf profile default to rpm and t...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1301#discussion_r144384241
--- Diff: pxf/build.gradle ---
@@ -310,6 +315,7 @@ project('pxf-service') {
project.distTar {
from('src/main/resources/pxf-profiles.xml') { into 'conf' }
+ from('src/main/resources/pxf-profiles-default.xml') { into 'conf' }
--- End diff --
Automation test failure is a different issue and that needs to be addressed separately out of this PR. This would not be an end user issue.
The automation test assumes pxf-profile.xml has all the profiles in it which is not really the case anymore. The tests need to addressed separately.
---
[GitHub] incubator-hawq pull request #1301: Re-added pxf profile default to rpm and t...
Posted by lavjain <gi...@git.apache.org>.
Github user lavjain commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1301#discussion_r144386624
--- Diff: pxf/build.gradle ---
@@ -310,6 +315,7 @@ project('pxf-service') {
project.distTar {
from('src/main/resources/pxf-profiles.xml') { into 'conf' }
+ from('src/main/resources/pxf-profiles-default.xml') { into 'conf' }
--- End diff --
I still do not understand why pxf-profile.xml needs to be empty but that is a product question for @kongyew
---
[GitHub] incubator-hawq issue #1301: Re-added pxf profile default to rpm and tar task...
Posted by lavjain <gi...@git.apache.org>.
Github user lavjain commented on the issue:
https://github.com/apache/incubator-hawq/pull/1301
If pxf-private-default is not needed, then we should remove it. The less number of conf files we have, the better.
---