Mailing-List: contact dev-help@hawq.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hawq.incubator.apache.org
From: sansanichfb <git@git.apache.org>
To: dev@hawq.incubator.apache.org
Reply-To: dev@hawq.incubator.apache.org
References: <git-pr-522-incubator-hawq@git.apache.org>
In-Reply-To: <git-pr-522-incubator-hawq@git.apache.org>
Subject: [GitHub] incubator-hawq pull request: HAWQ-577. Updated PXF metadata
 api to...
Content-Type: text/plain
Message-Id: <20160329015554.847C5DFC73@git1-us-west.apache.org>
Date: Tue, 29 Mar 2016 01:55:54 +0000 (UTC)

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.
---