You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pirk.apache.org by jryancarr <gi...@git.apache.org> on 2016/09/29 15:22:17 UTC

[GitHub] incubator-pirk pull request #105: Pirk 71

GitHub user jryancarr opened a pull request:

    https://github.com/apache/incubator-pirk/pull/105

    Pirk 71

    Added QuerierFactory and EncryptionPropertiesBuilder classes, which should make it easier to create a Querier object and return it directly to the caller, rather than having to write it to disk. This will make it easier for other projects to generate PIRK queries and pass them around.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jryancarr/incubator-pirk PIRK-71

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-pirk/pull/105.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 #105
    
----
commit 363c600e5b24d98dd72dd9504f54ff60d98b79d5
Author: jryancarr <jr...@gmail.com>
Date:   2016-09-28T19:24:59Z

    -Added QuerierFactory
    -Added EncryptionPropertiesBuilder to make it easier to construct valid
    properties files for the QuerierFactory
    -Refactored the QuerierDriver to call QuerierFactory.

commit 74f9b16d7b844f282960e60a01f0339a7862ec55
Author: jryancarr <jr...@gmail.com>
Date:   2016-09-28T19:27:41Z

    Forgot license files.

commit 64854c2e67c7d4e14d793d475ceba97052b7086c
Author: jryancarr <jr...@gmail.com>
Date:   2016-09-29T15:14:56Z

    Incorporated QuerierFactory into StandaloneQuery, so these changes
    are tested as part of the rest of the standalone test suite.

----


---
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-pirk pull request #105: [PIRK 71] Move Querier creation logic into...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-pirk/pull/105


---
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-pirk issue #105: [PIRK 71] Move Querier creation logic into Querie...

Posted by wraydulany <gi...@git.apache.org>.
Github user wraydulany commented on the issue:

    https://github.com/apache/incubator-pirk/pull/105
  
    +1 Works for me.


---
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-pirk pull request #105: [PIRK 71] Move Querier creation logic into...

Posted by jryancarr <gi...@git.apache.org>.
Github user jryancarr commented on a diff in the pull request:

    https://github.com/apache/incubator-pirk/pull/105#discussion_r81339999
  
    --- Diff: src/main/java/org/apache/pirk/querier/wideskies/QuerierCLI.java ---
    @@ -131,12 +125,28 @@ private boolean parseOptions()
         // Validate properties
         valid = QuerierProps.validateQuerierProperties();
     
    +    // Load the new local query and data schemas
    +    if (valid)
    +    {
    +      logger.info("loading schemas: dataSchemas = " + SystemConfiguration.getProperty("data.schemas") + " querySchemas = " + SystemConfiguration
    +          .getProperty("query.schemas"));
    +      try
    +      {
    +        DataSchemaLoader.initialize();
    +        QuerySchemaLoader.initialize();
    +
    +      } catch (Exception e)
    --- End diff --
    
    This code was moved out of the old QuerierProps.validateQuerierProperties() method. I decided it would be safe to move out since this was the only place that method was ever called, and this code doing initialization, not validation. But I'm not familiar enough yet with what the schema loaders are doing to understand how to handle those exceptions differently. Any suggestions?


---
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-pirk pull request #105: [PIRK 71] Move Querier creation logic into...

Posted by jryancarr <gi...@git.apache.org>.
Github user jryancarr commented on a diff in the pull request:

    https://github.com/apache/incubator-pirk/pull/105#discussion_r81340968
  
    --- Diff: src/main/java/org/apache/pirk/querier/wideskies/QuerierFactory.java ---
    @@ -0,0 +1,115 @@
    +/*
    + * 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.
    + */
    +package org.apache.pirk.querier.wideskies;
    +
    +import org.apache.pirk.encryption.Paillier;
    +import org.apache.pirk.querier.wideskies.encrypt.EncryptQuery;
    +import org.apache.pirk.query.wideskies.QueryInfo;
    +import org.apache.pirk.schema.query.QuerySchemaRegistry;
    +import org.apache.pirk.utils.PIRException;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.math.BigInteger;
    +import java.util.List;
    +import java.util.Properties;
    +import java.util.UUID;
    +
    +/**
    + * Handles encrypting a query and constructing a {@link Querier} given a {@link EncryptionPropertiesBuilder}.
    + *
    + * Created by ryan on 9/28/16.
    --- End diff --
    
    Sorry! Will be more careful next time. Looks like @ellisonanne took care of this already as well.


---
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-pirk pull request #105: [PIRK 71] Move Querier creation logic into...

Posted by tellison <gi...@git.apache.org>.
Github user tellison commented on a diff in the pull request:

    https://github.com/apache/incubator-pirk/pull/105#discussion_r81305311
  
    --- Diff: src/main/java/org/apache/pirk/querier/wideskies/QuerierFactory.java ---
    @@ -0,0 +1,115 @@
    +/*
    + * 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.
    + */
    +package org.apache.pirk.querier.wideskies;
    +
    +import org.apache.pirk.encryption.Paillier;
    +import org.apache.pirk.querier.wideskies.encrypt.EncryptQuery;
    +import org.apache.pirk.query.wideskies.QueryInfo;
    +import org.apache.pirk.schema.query.QuerySchemaRegistry;
    +import org.apache.pirk.utils.PIRException;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.math.BigInteger;
    +import java.util.List;
    +import java.util.Properties;
    +import java.util.UUID;
    +
    +/**
    + * Handles encrypting a query and constructing a {@link Querier} given a {@link EncryptionPropertiesBuilder}.
    + *
    + * Created by ryan on 9/28/16.
    --- End diff --
    
    Please drop author info in comments.


---
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-pirk pull request #105: [PIRK 71] Move Querier creation logic into...

Posted by tellison <gi...@git.apache.org>.
Github user tellison commented on a diff in the pull request:

    https://github.com/apache/incubator-pirk/pull/105#discussion_r81306398
  
    --- Diff: src/main/java/org/apache/pirk/test/utils/StandaloneQuery.java ---
    @@ -77,25 +76,16 @@
         logger.info("fileQuerier = " + fileQuerier.getAbsolutePath() + " fileQuery  = " + fileQuery.getAbsolutePath() + " responseFile = "
             + fileResponse.getAbsolutePath() + " fileFinalResults = " + fileFinalResults.getAbsolutePath());
     
    -    boolean embedSelector = SystemConfiguration.getBooleanProperty("pirTest.embedSelector", false);
    -    boolean useExpLookupTable = SystemConfiguration.getBooleanProperty("pirTest.useExpLookupTable", false);
    -    boolean useHDFSExpLookupTable = SystemConfiguration.getBooleanProperty("pirTest.useHDFSExpLookupTable", false);
    -
    -    // Set the necessary objects
    -    QueryInfo queryInfo = new QueryInfo(BaseTests.queryIdentifier, selectors.size(), BaseTests.hashBitSize, BaseTests.hashKey, BaseTests.dataPartitionBitSize,
    -        queryType, useExpLookupTable, embedSelector, useHDFSExpLookupTable);
    -
    -    if (SystemConfiguration.getBooleanProperty("pir.embedQuerySchema", false))
    -    {
    -      queryInfo.addQuerySchema(qSchema);
    -    }
    -
    -    Paillier paillier = new Paillier(BaseTests.paillierBitSize, BaseTests.certainty);
    +    Properties baseTestEncryptionProperties = EncryptionPropertiesBuilder.newBuilder()
    +          .dataPartitionBitSize(BaseTests.dataPartitionBitSize)
    +          .hashBitSize(BaseTests.hashBitSize)
    +          .hashKey(BaseTests.hashKey)
    +          .paillierBitSize(BaseTests.paillierBitSize)
    +          .certainty(BaseTests.certainty)
    +          .queryType(queryType)
    +          .build();
    --- End diff --
    
    Nice.


---
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-pirk pull request #105: [PIRK 71] Move Querier creation logic into...

Posted by tellison <gi...@git.apache.org>.
Github user tellison commented on a diff in the pull request:

    https://github.com/apache/incubator-pirk/pull/105#discussion_r81304556
  
    --- Diff: src/main/java/org/apache/pirk/querier/wideskies/EncryptionPropertiesBuilder.java ---
    @@ -0,0 +1,110 @@
    +/*
    + * 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.
    + */
    +package org.apache.pirk.querier.wideskies;
    +
    +import org.apache.pirk.utils.PIRException;
    +
    +import java.util.Properties;
    +
    +import static org.apache.pirk.querier.wideskies.QuerierProps.*;
    +
    +/**
    + * Holds the various parameters related to creating a {@link Querier}.
    + *
    + * Created by ryan on 9/28/16.
    --- End diff --
    
    Please drop the author info.


---
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-pirk pull request #105: [PIRK 71] Move Querier creation logic into...

Posted by tellison <gi...@git.apache.org>.
Github user tellison commented on a diff in the pull request:

    https://github.com/apache/incubator-pirk/pull/105#discussion_r81305924
  
    --- Diff: src/main/java/org/apache/pirk/querier/wideskies/QuerierProps.java ---
    @@ -60,154 +59,179 @@
       // Decryption properties
       static final String QUERIERFILE = "querier.querierFile";
     
    -  static final List<String> PROPSLIST = Arrays.asList(ACTION, INPUTFILE, OUTPUTFILE, QUERYTYPE, NUMTHREADS, EMBEDQUERYSCHEMA, HASHBITSIZE, HASHKEY,
    -      DATAPARTITIONSIZE, PAILLIERBITSIZE, BITSET, CERTAINTY, QUERYSCHEMAS, DATASCHEMAS, EMBEDSELECTOR, USEMEMLOOKUPTABLE, USEHDFSLOOKUPTABLE, SR_ALGORITHM,
    -      SR_PROVIDER);
    +  static final List<String> PROPSLIST = Arrays
    +      .asList(ACTION, INPUTFILE, OUTPUTFILE, QUERYTYPE, NUMTHREADS, EMBEDQUERYSCHEMA, HASHBITSIZE, HASHKEY, DATAPARTITIONSIZE, PAILLIERBITSIZE, BITSET,
    +          CERTAINTY, QUERYSCHEMAS, DATASCHEMAS, EMBEDSELECTOR, USEMEMLOOKUPTABLE, USEHDFSLOOKUPTABLE, SR_ALGORITHM, SR_PROVIDER);
     
    -  /**
    -   * Validates the querier properties
    -   *
    -   */
       public static boolean validateQuerierProperties()
       {
    +    setGeneralDefaults(SystemConfiguration.getProperties());
    +    if(validateGeneralQuerierProperties(SystemConfiguration.getProperties())) {
    +      String action = SystemConfiguration.getProperty(ACTION).toLowerCase();
    --- End diff --
    
    Trivial: does not follow house formatting rules ;-) 
    I believe this has subsequently been fixed by @ellisonanne 


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

Re: [GitHub] incubator-pirk pull request #105: [PIRK 71] Move Querier creation logic into...

Posted by Ellison Anne Williams <ea...@apache.org>.
I will take off the author info and push - I meant to do this yesterday
when I reformatted...

On Fri, Sep 30, 2016 at 5:31 AM, tellison <gi...@git.apache.org> wrote:

> Github user tellison commented on a diff in the pull request:
>
>     https://github.com/apache/incubator-pirk/pull/105#discussion_r81305132
>
>     --- Diff: src/main/java/org/apache/pirk/querier/wideskies/QuerierCLI.java
> ---
>     @@ -131,12 +125,28 @@ private boolean parseOptions()
>          // Validate properties
>          valid = QuerierProps.validateQuerierProperties();
>
>     +    // Load the new local query and data schemas
>     +    if (valid)
>     +    {
>     +      logger.info("loading schemas: dataSchemas = " +
> SystemConfiguration.getProperty("data.schemas") + " querySchemas = " +
> SystemConfiguration
>     +          .getProperty("query.schemas"));
>     +      try
>     +      {
>     +        DataSchemaLoader.initialize();
>     +        QuerySchemaLoader.initialize();
>     +
>     +      } catch (Exception e)
>     --- End diff --
>
>     It would be much better to handle the exception, or re-throw it,
> rather than consume and continue.
>     Why catch all Exception types?
>
>
> ---
> 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-pirk pull request #105: [PIRK 71] Move Querier creation logic into...

Posted by tellison <gi...@git.apache.org>.
Github user tellison commented on a diff in the pull request:

    https://github.com/apache/incubator-pirk/pull/105#discussion_r81305132
  
    --- Diff: src/main/java/org/apache/pirk/querier/wideskies/QuerierCLI.java ---
    @@ -131,12 +125,28 @@ private boolean parseOptions()
         // Validate properties
         valid = QuerierProps.validateQuerierProperties();
     
    +    // Load the new local query and data schemas
    +    if (valid)
    +    {
    +      logger.info("loading schemas: dataSchemas = " + SystemConfiguration.getProperty("data.schemas") + " querySchemas = " + SystemConfiguration
    +          .getProperty("query.schemas"));
    +      try
    +      {
    +        DataSchemaLoader.initialize();
    +        QuerySchemaLoader.initialize();
    +
    +      } catch (Exception e)
    --- End diff --
    
    It would be much better to handle the exception, or re-throw it, rather than consume and continue.
    Why catch all Exception types?


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