You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by okram <gi...@git.apache.org> on 2015/10/13 19:35:25 UTC

[GitHub] incubator-tinkerpop pull request: TINKERPOP3-843

GitHub user okram opened a pull request:

    https://github.com/apache/incubator-tinkerpop/pull/106

    TINKERPOP3-843

    Test cases passed locally.

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

    $ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP3-843

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

    https://github.com/apache/incubator-tinkerpop/pull/106.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 #106
    
----
commit 6c87b0d0996bd9d0e41b7838ef7df202a0f974e9
Author: Marko A. Rodriguez <ok...@gmail.com>
Date:   2015-10-13T17:25:34Z

    If the user provides a bad directory name for HADOOP_GRELMIN_LIBS there is a user WARN message that the directory does not exist. Added a test case that validates graceful behavior. Also, figured out how to reuse tests across HadoopGremlin extenders (i.e. Spark and Giraph).

----


---
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-tinkerpop pull request: TINKERPOP3-843

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

    https://github.com/apache/incubator-tinkerpop/pull/106#discussion_r41981371
  
    --- Diff: spark-gremlin/src/test/java/org/apache/tinkerpop/gremlin/spark/process/computer/SparkHadoopGraphProvider.java ---
    @@ -0,0 +1,53 @@
    +/*
    + * 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.tinkerpop.gremlin.spark.process.computer;
    +
    +import org.apache.tinkerpop.gremlin.GraphProvider;
    +import org.apache.tinkerpop.gremlin.LoadGraphWith;
    +import org.apache.tinkerpop.gremlin.hadoop.HadoopGraphProvider;
    +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
    +import org.apache.tinkerpop.gremlin.process.traversal.engine.ComputerTraversalEngine;
    +import org.apache.tinkerpop.gremlin.structure.Graph;
    +
    +import java.util.Map;
    +
    +/**
    + * @author Marko A. Rodriguez (http://markorodriguez.com)
    + */
    +@GraphProvider.Descriptor(computer = SparkGraphComputer.class)
    +public final class SparkHadoopGraphProvider extends HadoopGraphProvider {
    +
    +    @Override
    +    public Map<String, Object> getBaseConfiguration(final String graphName, final Class<?> test, final String testMethodName, final LoadGraphWith.GraphData loadGraphWith) {
    +        final Map<String, Object> config = super.getBaseConfiguration(graphName, test, testMethodName, loadGraphWith);
    +        config.put("mapreduce.job.reduces", 4);
    +        /// spark configuration
    +        config.put("spark.master", "local[4]");
    --- End diff --
    
    I think `local[*]` would make a better default value (`[*]` will use as many CPUs as available, `[4]` will always use 4, regardless of how many CPUs are actually available).


---
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-tinkerpop pull request: TINKERPOP3-843

Posted by okram <gi...@git.apache.org>.
Github user okram commented on the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/106#issuecomment-147787966
  
    I VOTE +1. I ran the test suite and checked Spark/Giraph integration tests just for a "few tests" to make sure it was all connected correctly.


---
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-tinkerpop pull request: TINKERPOP3-843

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

    https://github.com/apache/incubator-tinkerpop/pull/106#discussion_r41991520
  
    --- Diff: spark-gremlin/src/test/java/org/apache/tinkerpop/gremlin/spark/process/computer/SparkHadoopGraphProvider.java ---
    @@ -0,0 +1,53 @@
    +/*
    + * 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.tinkerpop.gremlin.spark.process.computer;
    +
    +import org.apache.tinkerpop.gremlin.GraphProvider;
    +import org.apache.tinkerpop.gremlin.LoadGraphWith;
    +import org.apache.tinkerpop.gremlin.hadoop.HadoopGraphProvider;
    +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
    +import org.apache.tinkerpop.gremlin.process.traversal.engine.ComputerTraversalEngine;
    +import org.apache.tinkerpop.gremlin.structure.Graph;
    +
    +import java.util.Map;
    +
    +/**
    + * @author Marko A. Rodriguez (http://markorodriguez.com)
    + */
    +@GraphProvider.Descriptor(computer = SparkGraphComputer.class)
    +public final class SparkHadoopGraphProvider extends HadoopGraphProvider {
    +
    +    @Override
    +    public Map<String, Object> getBaseConfiguration(final String graphName, final Class<?> test, final String testMethodName, final LoadGraphWith.GraphData loadGraphWith) {
    +        final Map<String, Object> config = super.getBaseConfiguration(graphName, test, testMethodName, loadGraphWith);
    +        config.put("mapreduce.job.reduces", 4);
    +        /// spark configuration
    +        config.put("spark.master", "local[4]");
    --- End diff --
    
    This is a test case and I want things to run fast, but still test threading. Dedicating to many threads is an overdose.
    
    Note that this ticket has nothing to do with this. You see this as "new" cause I renamed the test case class. It has always been `local[4]`. You can make a new ticket if you think that we should change this.


---
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-tinkerpop pull request: TINKERPOP3-843

Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/106#issuecomment-148075548
  
    +1, after inspecting the fraction of the committed code that was actually relevant to resolve the ticket.
    
    Congrats, that's the 3rd +1, which means you can finally merge.


---
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-tinkerpop pull request: TINKERPOP3-843

Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/106#issuecomment-147834516
  
    +1 - standard test suite is happy and did a manual test (the message i was looking for is present):
    
    ```text
    gremlin> graph = GraphFactory.open('conf/hadoop/hadoop-gryo.properties')
    ==>hadoopgraph[gryoinputformat->gryooutputformat]
    gremlin> g = graph.traversal(computer(SparkGraphComputer))
    ==>graphtraversalsource[hadoopgraph[gryoinputformat->gryooutputformat], sparkgraphcomputer]
    gremlin> g.V().count()
    ...
    WARN  org.apache.tinkerpop.gremlin.spark.process.computer.SparkGraphComputer  - /home/graphie/titan09/titan/lib/hadoop-gremlin-3.0.1-SNAPSHOT.jar does not reference a valid directory -- proceeding regardless
    ==>6
    ```


---
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-tinkerpop pull request: TINKERPOP3-843

Posted by ds-jenkins-builds <gi...@git.apache.org>.
Github user ds-jenkins-builds commented on the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/106#issuecomment-147794814
  
    Build finished. No test results found.



---
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-tinkerpop pull request: TINKERPOP3-843

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

    https://github.com/apache/incubator-tinkerpop/pull/106


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