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 2016/03/29 01:32:14 UTC
[GitHub] incubator-hawq pull request: HAWQ-577. Updated PXF metadata api to...
GitHub user shivzone opened a pull request:
https://github.com/apache/incubator-hawq/pull/522
HAWQ-577. Updated PXF metadata api to stream response
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-hawq HAWQ-577
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/522.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 #522
----
commit 68e30075976aff3e4b79dca08d5033e99c901b07
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-03-28T23:31:34Z
HAWQ-577. Updated PXF metadata api to stream response
----
---
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-577. Updated PXF metadata api to...
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on the pull request:
https://github.com/apache/incubator-hawq/pull/522#issuecomment-202643273
+1. looks 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-577. Updated PXF metadata api to...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone closed the pull request at:
https://github.com/apache/incubator-hawq/pull/522
---
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-577. Updated PXF metadata api to...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on the pull request:
https://github.com/apache/incubator-hawq/pull/522#issuecomment-202665815
LGTM
---
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-577. Updated PXF metadata api to...
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/522#discussion_r57656665
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/MetadataResponse.java ---
@@ -0,0 +1,94 @@
+package org.apache.hawq.pxf.service;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.List;
+
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.StreamingOutput;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hawq.pxf.api.Metadata;
+
+import org.codehaus.jackson.map.ObjectMapper;
+import org.codehaus.jackson.map.annotate.JsonSerialize.Inclusion;
+
+
+/**
+ * Class for serializing metadata in JSON format. The class implements
+ * {@link StreamingOutput} so the serialization will be done in a stream and not
+ * in one bulk, this in order to avoid running out of memory when processing a
+ * lot of items.
+ */
+public class MetadataResponse implements StreamingOutput {
+
+ private static final Log Log = LogFactory.getLog(MetadataResponse.class);
+ private static final String METADATA_DEFAULT_RESPONSE = "{\"PXFMetadata\":[]}";
+
+ private List<Metadata> metadataList;
+
+ /**
+ * Constructs metadata response out of a metadata list
+ *
+ * @param metadataList metadata list
+ */
+ public MetadataResponse(List<Metadata> metadataList) {
+ this.metadataList = metadataList;
+ }
+
+ /**
+ * Serializes the metadata list in JSON, To be used as the result string for HAWQ.
+ */
+ @Override
+ public void write(OutputStream output) throws IOException,
+ WebApplicationException {
+ DataOutputStream dos = new DataOutputStream(output);
+ ObjectMapper mapper = new ObjectMapper();
+ mapper.setSerializationInclusion(Inclusion.NON_EMPTY); // ignore empty fields
+
+ if(metadataList == null || metadataList.isEmpty()) {
+ dos.write(METADATA_DEFAULT_RESPONSE.getBytes());
+ return;
+ }
+
+ dos.write("{\"PXFMetadata\":[".getBytes());
+
+ String prefix = "";
+ for (Metadata metadata : metadataList) {
+ if(metadata == null) {
+ throw new IllegalArgumentException("metadata object is null - cannot serialize");
+ }
+ if ((metadata.getFields() == null) || metadata.getFields().isEmpty()) {
+ throw new IllegalArgumentException("metadata for " + metadata.getItem() + " contains no fields - cannot serialize");
+ }
+ StringBuilder result = new StringBuilder();
+ /* metaData and userData are automatically converted to Base64 */
--- End diff --
probably remove comment :)
---
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-577. Updated PXF metadata api to...
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/522#discussion_r57661884
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/MetadataResponse.java ---
@@ -0,0 +1,94 @@
+package org.apache.hawq.pxf.service;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.List;
+
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.StreamingOutput;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hawq.pxf.api.Metadata;
+
+import org.codehaus.jackson.map.ObjectMapper;
+import org.codehaus.jackson.map.annotate.JsonSerialize.Inclusion;
+
+
+/**
+ * Class for serializing metadata in JSON format. The class implements
+ * {@link StreamingOutput} so the serialization will be done in a stream and not
+ * in one bulk, this in order to avoid running out of memory when processing a
+ * lot of items.
+ */
+public class MetadataResponse implements StreamingOutput {
+
+ private static final Log Log = LogFactory.getLog(MetadataResponse.class);
+ private static final String METADATA_DEFAULT_RESPONSE = "{\"PXFMetadata\":[]}";
+
+ private List<Metadata> metadataList;
+
+ /**
+ * Constructs metadata response out of a metadata list
+ *
+ * @param metadataList metadata list
+ */
+ public MetadataResponse(List<Metadata> metadataList) {
+ this.metadataList = metadataList;
+ }
+
+ /**
+ * Serializes the metadata list in JSON, To be used as the result string for HAWQ.
+ */
+ @Override
+ public void write(OutputStream output) throws IOException,
+ WebApplicationException {
+ DataOutputStream dos = new DataOutputStream(output);
+ ObjectMapper mapper = new ObjectMapper();
+ mapper.setSerializationInclusion(Inclusion.NON_EMPTY); // ignore empty fields
+
+ if(metadataList == null || metadataList.isEmpty()) {
+ dos.write(METADATA_DEFAULT_RESPONSE.getBytes());
+ return;
+ }
+
+ dos.write("{\"PXFMetadata\":[".getBytes());
--- End diff --
What i meant was I wouldn't want to have any coorelation between metadata_default_response and the header string used in the actual response. I wouldn't mind introducing a separate constant for the string used in the acutal response but wouldn't use that as part of the default response
---
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-577. Updated PXF metadata api to...
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/522#discussion_r57654541
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/MetadataResponseFormatter.java ---
@@ -46,76 +45,50 @@
* @return JSON formatted response
* @throws IOException if converting the data to JSON fails
*/
- public static String formatResponseString(List<Metadata> metadataList) throws IOException {
- return MetadataResponseFormatter.metadataToJSON(metadataList);
- }
-
- /**
- * Serializes a metadata in JSON,
- * To be used as the result string for HAWQ.
- * An example result is as follows:
- *
- * {"PXFMetadata":[{"item":{"path":"default","name":"t1"},"fields":[{"name":"a","type":"int"},{"name":"b","type":"float"}]}]}
- */
- private static String metadataToJSON(List<Metadata> metadataList) throws IOException {
-
- if (metadataList == null || metadataList.isEmpty()) {
- return METADATA_DEFAULT_RESPONSE;
+ public static MetadataResponse formatResponse(List<Metadata> metadataList, String path) throws IOException {
+ /* print the fragment list to log when in debug level */
+ if (LOG.isDebugEnabled()) {
+ MetadataResponseFormatter.printMetadata(metadataList, path);
}
- StringBuilder result = null;
-
- for(Metadata metadata: metadataList) {
- if(metadata == null) {
- throw new IllegalArgumentException("metadata object is null - cannot serialize");
- }
- if ((metadata.getFields() == null) || metadata.getFields().isEmpty()) {
- throw new IllegalArgumentException("metadata for " + metadata.getItem() + " contains no fields - cannot serialize");
- }
- if (result == null) {
- result = new StringBuilder("{\"PXFMetadata\":["); /* prefix info */
- } else {
- result.append(",");
- }
-
- ObjectMapper mapper = new ObjectMapper();
- mapper.setSerializationInclusion(Inclusion.NON_EMPTY); // ignore empty fields
- result.append(mapper.writeValueAsString(metadata));
- }
-
- return result.append("]}").toString(); /* append suffix info */
-
+ return new MetadataResponse(metadataList);
}
/**
* Converts metadata list to a readable string.
* Intended for debugging purposes only.
*/
- private static String metadataToString(List<Metadata> metadataList) {
- StringBuilder result = new StringBuilder("Metadata:");
+ private static void printMetadata(List<Metadata> metadataList, String path) {
+ LOG.debug("Metadata List for path " + path + ": ");
+
+ if (null == metadataList || metadataList.isEmpty()) {
+ LOG.debug("No metadata");
+ return;
+ }
for(Metadata metadata: metadataList) {
- result.append(" Metadata for item \"");
+ StringBuilder result = new StringBuilder();
+ result.append("Metadata for item \"");
--- End diff --
the quotes are not closed for the case of null metadata.
perhaps move `\"` to be part of line 79.
---
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-577. Updated PXF metadata api to...
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/522#discussion_r57656799
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/MetadataResponse.java ---
@@ -0,0 +1,94 @@
+package org.apache.hawq.pxf.service;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.List;
+
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.StreamingOutput;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hawq.pxf.api.Metadata;
+
+import org.codehaus.jackson.map.ObjectMapper;
+import org.codehaus.jackson.map.annotate.JsonSerialize.Inclusion;
+
+
+/**
+ * Class for serializing metadata in JSON format. The class implements
+ * {@link StreamingOutput} so the serialization will be done in a stream and not
+ * in one bulk, this in order to avoid running out of memory when processing a
+ * lot of items.
+ */
+public class MetadataResponse implements StreamingOutput {
+
+ private static final Log Log = LogFactory.getLog(MetadataResponse.class);
+ private static final String METADATA_DEFAULT_RESPONSE = "{\"PXFMetadata\":[]}";
+
+ private List<Metadata> metadataList;
+
+ /**
+ * Constructs metadata response out of a metadata list
+ *
+ * @param metadataList metadata list
+ */
+ public MetadataResponse(List<Metadata> metadataList) {
+ this.metadataList = metadataList;
+ }
+
+ /**
+ * Serializes the metadata list in JSON, To be used as the result string for HAWQ.
+ */
+ @Override
+ public void write(OutputStream output) throws IOException,
+ WebApplicationException {
+ DataOutputStream dos = new DataOutputStream(output);
+ ObjectMapper mapper = new ObjectMapper();
+ mapper.setSerializationInclusion(Inclusion.NON_EMPTY); // ignore empty fields
+
+ if(metadataList == null || metadataList.isEmpty()) {
+ dos.write(METADATA_DEFAULT_RESPONSE.getBytes());
+ return;
+ }
+
+ dos.write("{\"PXFMetadata\":[".getBytes());
--- End diff --
perhaps make PXFMetadata into a const, and use it here and as part of of METADATA_DEFAULT_RESPONSE.
---
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-577. Updated PXF metadata api to...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/522#discussion_r57662935
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/MetadataResponse.java ---
@@ -0,0 +1,94 @@
+package org.apache.hawq.pxf.service;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.List;
+
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.StreamingOutput;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hawq.pxf.api.Metadata;
+
+import org.codehaus.jackson.map.ObjectMapper;
+import org.codehaus.jackson.map.annotate.JsonSerialize.Inclusion;
+
+
+/**
+ * Class for serializing metadata in JSON format. The class implements
+ * {@link StreamingOutput} so the serialization will be done in a stream and not
+ * in one bulk, this in order to avoid running out of memory when processing a
+ * lot of items.
+ */
+public class MetadataResponse implements StreamingOutput {
+
+ private static final Log Log = LogFactory.getLog(MetadataResponse.class);
+ private static final String METADATA_DEFAULT_RESPONSE = "{\"PXFMetadata\":[]}";
+
+ private List<Metadata> metadataList;
+
+ /**
+ * Constructs metadata response out of a metadata list
+ *
+ * @param metadataList metadata list
+ */
+ public MetadataResponse(List<Metadata> metadataList) {
+ this.metadataList = metadataList;
+ }
+
+ /**
+ * Serializes the metadata list in JSON, To be used as the result string for HAWQ.
+ */
+ @Override
+ public void write(OutputStream output) throws IOException,
+ WebApplicationException {
+ DataOutputStream dos = new DataOutputStream(output);
--- End diff --
Should we close resource in the end?
---
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-577. Updated PXF metadata api to...
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/522#discussion_r57654925
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/MetadataResponseFormatter.java ---
@@ -46,76 +45,50 @@
* @return JSON formatted response
* @throws IOException if converting the data to JSON fails
*/
- public static String formatResponseString(List<Metadata> metadataList) throws IOException {
- return MetadataResponseFormatter.metadataToJSON(metadataList);
- }
-
- /**
- * Serializes a metadata in JSON,
- * To be used as the result string for HAWQ.
- * An example result is as follows:
- *
- * {"PXFMetadata":[{"item":{"path":"default","name":"t1"},"fields":[{"name":"a","type":"int"},{"name":"b","type":"float"}]}]}
- */
- private static String metadataToJSON(List<Metadata> metadataList) throws IOException {
-
- if (metadataList == null || metadataList.isEmpty()) {
- return METADATA_DEFAULT_RESPONSE;
+ public static MetadataResponse formatResponse(List<Metadata> metadataList, String path) throws IOException {
+ /* print the fragment list to log when in debug level */
+ if (LOG.isDebugEnabled()) {
+ MetadataResponseFormatter.printMetadata(metadataList, path);
}
- StringBuilder result = null;
-
- for(Metadata metadata: metadataList) {
- if(metadata == null) {
- throw new IllegalArgumentException("metadata object is null - cannot serialize");
- }
- if ((metadata.getFields() == null) || metadata.getFields().isEmpty()) {
- throw new IllegalArgumentException("metadata for " + metadata.getItem() + " contains no fields - cannot serialize");
- }
- if (result == null) {
- result = new StringBuilder("{\"PXFMetadata\":["); /* prefix info */
- } else {
- result.append(",");
- }
-
- ObjectMapper mapper = new ObjectMapper();
- mapper.setSerializationInclusion(Inclusion.NON_EMPTY); // ignore empty fields
- result.append(mapper.writeValueAsString(metadata));
- }
-
- return result.append("]}").toString(); /* append suffix info */
-
+ return new MetadataResponse(metadataList);
}
/**
* Converts metadata list to a readable string.
* Intended for debugging purposes only.
*/
- private static String metadataToString(List<Metadata> metadataList) {
- StringBuilder result = new StringBuilder("Metadata:");
+ private static void printMetadata(List<Metadata> metadataList, String path) {
+ LOG.debug("Metadata List for path " + path + ": ");
+
+ if (null == metadataList || metadataList.isEmpty()) {
+ LOG.debug("No metadata");
+ return;
+ }
for(Metadata metadata: metadataList) {
- result.append(" Metadata for item \"");
+ StringBuilder result = new StringBuilder();
+ result.append("Metadata for item \"");
--- End diff --
good catch
---
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-577. Updated PXF metadata api to...
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/522#discussion_r57654981
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/MetadataResource.java ---
@@ -41,6 +41,7 @@
import org.apache.hawq.pxf.api.MetadataFetcher;
import org.apache.hawq.pxf.api.utilities.InputData;
import org.apache.hawq.pxf.service.MetadataFetcherFactory;
+import org.apache.hawq.pxf.service.MetadataResponse;
--- End diff --
is this a new file?
---
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-577. Updated PXF metadata api to...
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/522#discussion_r57655549
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/MetadataResource.java ---
@@ -41,6 +41,7 @@
import org.apache.hawq.pxf.api.MetadataFetcher;
import org.apache.hawq.pxf.api.utilities.InputData;
import org.apache.hawq.pxf.service.MetadataFetcherFactory;
+import org.apache.hawq.pxf.service.MetadataResponse;
--- End diff --
yes, it is. I missed adding it and i've just updated
---
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-577. Updated PXF metadata api to...
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/522#discussion_r57657152
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/MetadataResponse.java ---
@@ -0,0 +1,94 @@
+package org.apache.hawq.pxf.service;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.List;
+
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.StreamingOutput;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hawq.pxf.api.Metadata;
+
+import org.codehaus.jackson.map.ObjectMapper;
+import org.codehaus.jackson.map.annotate.JsonSerialize.Inclusion;
+
+
+/**
+ * Class for serializing metadata in JSON format. The class implements
+ * {@link StreamingOutput} so the serialization will be done in a stream and not
+ * in one bulk, this in order to avoid running out of memory when processing a
+ * lot of items.
+ */
+public class MetadataResponse implements StreamingOutput {
+
+ private static final Log Log = LogFactory.getLog(MetadataResponse.class);
+ private static final String METADATA_DEFAULT_RESPONSE = "{\"PXFMetadata\":[]}";
+
+ private List<Metadata> metadataList;
+
+ /**
+ * Constructs metadata response out of a metadata list
+ *
+ * @param metadataList metadata list
+ */
+ public MetadataResponse(List<Metadata> metadataList) {
+ this.metadataList = metadataList;
+ }
+
+ /**
+ * Serializes the metadata list in JSON, To be used as the result string for HAWQ.
+ */
+ @Override
+ public void write(OutputStream output) throws IOException,
+ WebApplicationException {
+ DataOutputStream dos = new DataOutputStream(output);
+ ObjectMapper mapper = new ObjectMapper();
+ mapper.setSerializationInclusion(Inclusion.NON_EMPTY); // ignore empty fields
+
+ if(metadataList == null || metadataList.isEmpty()) {
+ dos.write(METADATA_DEFAULT_RESPONSE.getBytes());
+ return;
+ }
+
+ dos.write("{\"PXFMetadata\":[".getBytes());
--- End diff --
I didn't want to do that. The default response can possibly change tomorrow and wanted to decouple that from a valid response string.
---
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-577. Updated PXF metadata api to...
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/522#discussion_r57659746
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/MetadataResponse.java ---
@@ -0,0 +1,94 @@
+package org.apache.hawq.pxf.service;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.List;
+
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.StreamingOutput;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hawq.pxf.api.Metadata;
+
+import org.codehaus.jackson.map.ObjectMapper;
+import org.codehaus.jackson.map.annotate.JsonSerialize.Inclusion;
+
+
+/**
+ * Class for serializing metadata in JSON format. The class implements
+ * {@link StreamingOutput} so the serialization will be done in a stream and not
+ * in one bulk, this in order to avoid running out of memory when processing a
+ * lot of items.
+ */
+public class MetadataResponse implements StreamingOutput {
+
+ private static final Log Log = LogFactory.getLog(MetadataResponse.class);
+ private static final String METADATA_DEFAULT_RESPONSE = "{\"PXFMetadata\":[]}";
+
+ private List<Metadata> metadataList;
+
+ /**
+ * Constructs metadata response out of a metadata list
+ *
+ * @param metadataList metadata list
+ */
+ public MetadataResponse(List<Metadata> metadataList) {
+ this.metadataList = metadataList;
+ }
+
+ /**
+ * Serializes the metadata list in JSON, To be used as the result string for HAWQ.
+ */
+ @Override
+ public void write(OutputStream output) throws IOException,
+ WebApplicationException {
+ DataOutputStream dos = new DataOutputStream(output);
+ ObjectMapper mapper = new ObjectMapper();
+ mapper.setSerializationInclusion(Inclusion.NON_EMPTY); // ignore empty fields
+
+ if(metadataList == null || metadataList.isEmpty()) {
+ dos.write(METADATA_DEFAULT_RESPONSE.getBytes());
+ return;
+ }
+
+ dos.write("{\"PXFMetadata\":[".getBytes());
--- End diff --
You mean that the default might not contain the header "PXFMetadata"?
In that case I would do something like
```
final String METADATA_HEADER_RESPONSE = "PXFMetadata";
final String METADATA_DEFAULT_RESPONSE = "{\"" + METADATA_HEADER_RESPONSE + "\":[]}"
```
---
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-577. Updated PXF metadata api to...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/522#discussion_r57663034
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/MetadataResponseFormatter.java ---
@@ -46,76 +45,49 @@
* @return JSON formatted response
* @throws IOException if converting the data to JSON fails
*/
- public static String formatResponseString(List<Metadata> metadataList) throws IOException {
- return MetadataResponseFormatter.metadataToJSON(metadataList);
- }
-
- /**
- * Serializes a metadata in JSON,
- * To be used as the result string for HAWQ.
- * An example result is as follows:
- *
- * {"PXFMetadata":[{"item":{"path":"default","name":"t1"},"fields":[{"name":"a","type":"int"},{"name":"b","type":"float"}]}]}
- */
- private static String metadataToJSON(List<Metadata> metadataList) throws IOException {
-
- if (metadataList == null || metadataList.isEmpty()) {
- return METADATA_DEFAULT_RESPONSE;
+ public static MetadataResponse formatResponse(List<Metadata> metadataList, String path) throws IOException {
+ /* print the fragment list to log when in debug level */
+ if (LOG.isDebugEnabled()) {
+ MetadataResponseFormatter.printMetadata(metadataList, path);
}
- StringBuilder result = null;
-
- for(Metadata metadata: metadataList) {
- if(metadata == null) {
- throw new IllegalArgumentException("metadata object is null - cannot serialize");
- }
- if ((metadata.getFields() == null) || metadata.getFields().isEmpty()) {
- throw new IllegalArgumentException("metadata for " + metadata.getItem() + " contains no fields - cannot serialize");
- }
- if (result == null) {
- result = new StringBuilder("{\"PXFMetadata\":["); /* prefix info */
- } else {
- result.append(",");
- }
-
- ObjectMapper mapper = new ObjectMapper();
- mapper.setSerializationInclusion(Inclusion.NON_EMPTY); // ignore empty fields
- result.append(mapper.writeValueAsString(metadata));
- }
-
- return result.append("]}").toString(); /* append suffix info */
-
+ return new MetadataResponse(metadataList);
}
/**
* Converts metadata list to a readable string.
* Intended for debugging purposes only.
*/
- private static String metadataToString(List<Metadata> metadataList) {
- StringBuilder result = new StringBuilder("Metadata:");
+ private static void printMetadata(List<Metadata> metadataList, String path) {
+ LOG.debug("Metadata List for path " + path + ": ");
+
+ if (null == metadataList || metadataList.isEmpty()) {
+ LOG.debug("No metadata");
+ return;
+ }
for(Metadata metadata: metadataList) {
- result.append(" Metadata for item \"");
+ StringBuilder result = new StringBuilder();
if (metadata == null) {
- return "No metadata";
+ result.append("None");
--- End diff --
Literal "None" was used couple times, might me a constant.
---
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-577. Updated PXF metadata api to...
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/522#discussion_r57666661
--- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/MetadataResponseFormatter.java ---
@@ -46,76 +45,49 @@
* @return JSON formatted response
* @throws IOException if converting the data to JSON fails
*/
- public static String formatResponseString(List<Metadata> metadataList) throws IOException {
- return MetadataResponseFormatter.metadataToJSON(metadataList);
- }
-
- /**
- * Serializes a metadata in JSON,
- * To be used as the result string for HAWQ.
- * An example result is as follows:
- *
- * {"PXFMetadata":[{"item":{"path":"default","name":"t1"},"fields":[{"name":"a","type":"int"},{"name":"b","type":"float"}]}]}
- */
- private static String metadataToJSON(List<Metadata> metadataList) throws IOException {
-
- if (metadataList == null || metadataList.isEmpty()) {
- return METADATA_DEFAULT_RESPONSE;
+ public static MetadataResponse formatResponse(List<Metadata> metadataList, String path) throws IOException {
+ /* print the fragment list to log when in debug level */
+ if (LOG.isDebugEnabled()) {
+ MetadataResponseFormatter.printMetadata(metadataList, path);
}
- StringBuilder result = null;
-
- for(Metadata metadata: metadataList) {
- if(metadata == null) {
- throw new IllegalArgumentException("metadata object is null - cannot serialize");
- }
- if ((metadata.getFields() == null) || metadata.getFields().isEmpty()) {
- throw new IllegalArgumentException("metadata for " + metadata.getItem() + " contains no fields - cannot serialize");
- }
- if (result == null) {
- result = new StringBuilder("{\"PXFMetadata\":["); /* prefix info */
- } else {
- result.append(",");
- }
-
- ObjectMapper mapper = new ObjectMapper();
- mapper.setSerializationInclusion(Inclusion.NON_EMPTY); // ignore empty fields
- result.append(mapper.writeValueAsString(metadata));
- }
-
- return result.append("]}").toString(); /* append suffix info */
-
+ return new MetadataResponse(metadataList);
}
/**
* Converts metadata list to a readable string.
* Intended for debugging purposes only.
*/
- private static String metadataToString(List<Metadata> metadataList) {
- StringBuilder result = new StringBuilder("Metadata:");
+ private static void printMetadata(List<Metadata> metadataList, String path) {
+ LOG.debug("Metadata List for path " + path + ": ");
+
+ if (null == metadataList || metadataList.isEmpty()) {
+ LOG.debug("No metadata");
+ return;
+ }
for(Metadata metadata: metadataList) {
- result.append(" Metadata for item \"");
+ StringBuilder result = new StringBuilder();
if (metadata == null) {
- return "No metadata";
+ result.append("None");
--- End diff --
This is just used for logging and not for the response. Defining constants just for log message seems an overkill
---
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.
---