You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hawq.apache.org by shivzone <gi...@git.apache.org> on 2015/12/09 19:30:11 UTC
[GitHub] incubator-hawq pull request: HAWQ-191. Remove Analyzer plugin from...
GitHub user shivzone opened a pull request:
https://github.com/apache/incubator-hawq/pull/174
HAWQ-191. Remove Analyzer plugin from PXF
TBD
1. Handle gracefully if /analyzer api is used with a warning message
2. Automation test
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-hawq HAWQ-191
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/174.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 #174
----
commit 7b5532c8a587cc217fcebf0d29e19197188a1f51
Author: Shivram Mani <sh...@gmail.com>
Date: 2015-12-09T18:25:24Z
HAWQ-191. Remove Analyzer plugin from PXF
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-191. Remove Analyzer plugin from...
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/174#discussion_r47532428
--- Diff: pxf/build.gradle ---
@@ -218,7 +218,7 @@ project('pxf-service') {
project('pxf-hdfs') {
dependencies {
- compile(project(':pxf-service')) //Yikes, HdfsAnalyzer is directly accessing the bridge
+ compile(project(':pxf-service'))
--- End diff --
There are other compile time dependancies unrelated to the analyzer
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-191. Remove Analyzer plugin from...
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/174#discussion_r47574985
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/InvalidPathResource.java ---
@@ -153,9 +167,39 @@ private String parseVersion(String path) {
}
/**
+ * Parses the version part from the path.
--- End diff --
These are private methods used only by wrongPath(). Having an enum and separate function seems to be an overkill for this. will do away with helper functions and use pathSegments directly to extract the version and endpoint.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-191. Remove Analyzer plugin from...
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/174#discussion_r47579068
--- Diff: pxf/build.gradle ---
@@ -218,7 +218,7 @@ project('pxf-service') {
project('pxf-hdfs') {
dependencies {
- compile(project(':pxf-service')) //Yikes, HdfsAnalyzer is directly accessing the bridge
+ compile(project(':pxf-service'))
--- End diff --
Yes, separate jira makes more sense
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-191. Remove Analyzer plugin from...
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on the pull request:
https://github.com/apache/incubator-hawq/pull/174#issuecomment-164594300
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-191. Remove Analyzer plugin from...
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on the pull request:
https://github.com/apache/incubator-hawq/pull/174#issuecomment-163783470
Removed code looks good.
+0.5 (waiting for the warning message code)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-191. Remove Analyzer plugin from...
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/174#discussion_r47572138
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/InvalidPathResource.java ---
@@ -153,9 +167,39 @@ private String parseVersion(String path) {
}
/**
+ * Parses the version part from the path.
--- End diff --
1. Please update doc - it parses the part after the version part, right? /pxf/version/endpoint
2. could you please refactor parseVersion() to also use PathSegment?
Actually, both functions can perhaps be consolidated. [e.g. parsePathSegment(path, VERSION), parsePath(path, ENDPOINT)]
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-191. Remove Analyzer plugin from...
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/174#discussion_r47575698
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/InvalidPathResource.java ---
@@ -153,9 +167,39 @@ private String parseVersion(String path) {
}
/**
+ * Parses the version part from the path.
--- End diff --
Sounds good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-191. Remove Analyzer plugin from...
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/174#discussion_r47575757
--- Diff: pxf/build.gradle ---
@@ -218,7 +218,7 @@ project('pxf-service') {
project('pxf-hdfs') {
dependencies {
- compile(project(':pxf-service')) //Yikes, HdfsAnalyzer is directly accessing the bridge
+ compile(project(':pxf-service'))
--- End diff --
There are 2 other classes using pxf-service. I will open a JIRA to fix it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-191. Remove Analyzer plugin from...
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/174#discussion_r47581115
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/InvalidPathResource.java ---
@@ -153,9 +167,39 @@ private String parseVersion(String path) {
}
/**
+ * Parses the version part from the path.
--- End diff --
Fixed this directly on master
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-191. Remove Analyzer plugin from...
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/174#discussion_r47301589
--- Diff: pxf/build.gradle ---
@@ -218,7 +218,7 @@ project('pxf-service') {
project('pxf-hdfs') {
dependencies {
- compile(project(':pxf-service')) //Yikes, HdfsAnalyzer is directly accessing the bridge
+ compile(project(':pxf-service'))
--- End diff --
Can't we replace the dependency with a dependency on pxf-api (like pxf-hbase does)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-191. Remove Analyzer plugin from...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone closed the pull request at:
https://github.com/apache/incubator-hawq/pull/174
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---