RYA-253 Converted the rya.prospector class from Groovy to Java. Also added documentation to the project.#151
RYA-253 Converted the rya.prospector class from Groovy to Java. Also added documentation to the project.#151kchilton2 wants to merge 1 commit into
Conversation
| } catch (AccumuloSecurityException e) { | ||
| e.printStackTrace(); | ||
| } catch (final AccumuloException | AccumuloSecurityException | TableExistsException e) { | ||
| LOG.warn("A problem was encountered while setting the Configuration for the EntityOptimizer.", e); |
There was a problem hiding this comment.
there a way we can make this more informative?
There was a problem hiding this comment.
I don't know what else you'd say here. The chained exception will have the details.
| } catch (AccumuloSecurityException e) { | ||
| e.printStackTrace(); | ||
| } catch (final AccumuloException | AccumuloSecurityException | TableExistsException e) { | ||
| LOG.warn("A problem was encountered while constructing the EntityOptimizer.", e); |
There was a problem hiding this comment.
there a way we can make this more informative?
There was a problem hiding this comment.
I don't know what else you'd say here. The chained exception will have the details.
| import com.google.common.base.Joiner; | ||
|
|
||
| public class EntityTupleSet extends ExternalSet implements ExternalBatchingIterator { | ||
| /* |
There was a problem hiding this comment.
move the license to the top
| minCard = csp.getCardinality(); | ||
| minSp = csp.getSp(); | ||
| } else { | ||
| // TODO come up with a better default if cardinality is not |
There was a problem hiding this comment.
is this a 'make jira ticket' todo or can this be removed?
There was a problem hiding this comment.
I have no idea. This comment was already present when I was converting.
| } | ||
|
|
||
| @Override | ||
| public int compareTo(IntermediateProspect t) { |
There was a problem hiding this comment.
is this the norm for this kind of comparison? seems odd.
There was a problem hiding this comment.
Not sure. This is how it was already working so I'm not changing it.
| * The data portion of an {@link IndexEntry} contains a unique Subject that | ||
| * appears within a Rya instance's Statements. | ||
| */ | ||
| subject, |
There was a problem hiding this comment.
don't conventions put enums as all capitalized?
There was a problem hiding this comment.
Yea, I'll update the code.
There was a problem hiding this comment.
Just kidding, that breaks things because it uses the enum's value as part of the Row Key in accumulo. I don't want to change the functionality, so I'm leaving it.
There was a problem hiding this comment.
can't you override getValue() to return the lowercase?
There was a problem hiding this comment.
There is no getValue() method on enums. It's using toString() and valueOf(String). I would have to implement new methods on the enum to handle this case. Which I can do if this style is a deal breaker.
There was a problem hiding this comment.
I personally would like the extra functionality to keep to the convention
| assert outTable != null; | ||
| assert auths_str != null; | ||
|
|
||
| final Job job = new Job(getConf(), this.getClass().getSimpleName() + "_" + System.currentTimeMillis()); |
There was a problem hiding this comment.
I'm not changing the behavior of the code, I'm just converting it from Groovy to Java, so I'm going to leave this as is.
There was a problem hiding this comment.
then I don't think NOW is ever used
There was a problem hiding this comment.
It is used on lines 67 and 85.
| void setKey(K key) { | ||
| this.key = key | ||
| public void setKey(K key) { | ||
| this.key = key; |
There was a problem hiding this comment.
how was this even working without semicolons before.....
|
Update to reflect Andrew's comments. |
|
Changing the enums to all makes the unit tests fail because those enum values as strings are used within the storage, so I'm going to back out that change in my next update to this PR. |
|
|
|
asfbot build |
2 similar comments
|
asfbot build |
|
asfbot build |
|
Refer to this link for build results (access rights to CI server needed): Build result: FAILURE[...truncated 6.62 MB...][INFO] Apache Rya Web Projects ............................ SKIPPED[INFO] Apache Rya Web Implementation ...................... SKIPPED[INFO] ------------------------------------------------------------------------[JENKINS] Archiving disabled[INFO] BUILD FAILURE[INFO] ------------------------------------------------------------------------[INFO] Total time: 47:57 min[INFO] Finished at: 2017-04-11T18:08:47+00:00[INFO] Final Memory: 189M/996M[INFO] ------------------------------------------------------------------------Waiting for Jenkins to finish collecting data[ERROR] Failed to execute goal on project rya.geoindexing: Could not resolve dependencies for project org.apache.rya:rya.geoindexing:jar:3.2.11-incubating-SNAPSHOT: The following artifacts could not be resolved: mil.nga.giat:geowave-datastore-accumulo:jar:0.9.3, mil.nga.giat:geowave-adapter-vector:jar:0.9.3: Could not find artifact mil.nga.giat:geowave-datastore-accumulo:jar:0.9.3 in osgeo (http://download.osgeo.org/webdav/geotools/) -> [Help 1][ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.[ERROR] Re-run Maven using the -X switch to enable full debug logging.[ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles:[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/DependencyResolutionException[ERROR] [ERROR] After correcting the problems, you can resume the build with the command[ERROR] mvn -rf :rya.geoindexingchannel stoppedSetting status of a6c904c to FAILURE with url https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests-on-demand/44/ and message: 'Build finished. 'Using context: asfbot build |
|
asfbot build |
|
Refer to this link for build results (access rights to CI server needed): Build result: FAILURE[...truncated 5.84 MB...][INFO] Apache Rya Spark Support ........................... SKIPPED[INFO] Apache Rya Web Projects ............................ SKIPPED[INFO] Apache Rya Web Implementation ...................... SKIPPED[INFO] ------------------------------------------------------------------------[INFO] BUILD FAILURE[INFO] ------------------------------------------------------------------------[INFO] Total time: 27:41 min[INFO] Finished at: 2017-04-11T18:39:11+00:00[INFO] Final Memory: 250M/1144M[INFO] ------------------------------------------------------------------------Waiting for Jenkins to finish collecting data[ERROR] Failed to execute goal on project rya.geoindexing: Could not resolve dependencies for project org.apache.rya:rya.geoindexing:jar:3.2.11-incubating-SNAPSHOT: The following artifacts could not be resolved: mil.nga.giat:geowave-datastore-accumulo:jar:0.9.3, mil.nga.giat:geowave-adapter-vector:jar:0.9.3: Failure to find mil.nga.giat:geowave-datastore-accumulo:jar:0.9.3 in http://download.osgeo.org/webdav/geotools/ was cached in the local repository, resolution will not be reattempted until the update interval of osgeo has elapsed or updates are forced -> [Help 1][ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.[ERROR] Re-run Maven using the -X switch to enable full debug logging.[ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles:[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/DependencyResolutionException[ERROR] [ERROR] After correcting the problems, you can resume the build with the command[ERROR] mvn -rf :rya.geoindexingchannel stoppedSetting status of a6c904c to FAILURE with url https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests-on-demand/45/ and message: 'Build finished. 'Using context: asfbot build |
|
Refer to this link for build results (access rights to CI server needed): Build result: FAILURE[...truncated 5.83 MB...][INFO] Apache Rya Spark Support ........................... SKIPPED[INFO] Apache Rya Web Projects ............................ SKIPPED[INFO] Apache Rya Web Implementation ...................... SKIPPED[INFO] ------------------------------------------------------------------------[INFO] BUILD FAILURE[INFO] ------------------------------------------------------------------------[INFO] Total time: 28:02 min[INFO] Finished at: 2017-04-11T19:11:33+00:00[INFO] Final Memory: 242M/987M[INFO] ------------------------------------------------------------------------Waiting for Jenkins to finish collecting data[ERROR] Failed to execute goal on project rya.geoindexing: Could not resolve dependencies for project org.apache.rya:rya.geoindexing:jar:3.2.11-incubating-SNAPSHOT: The following artifacts could not be resolved: mil.nga.giat:geowave-datastore-accumulo:jar:0.9.3, mil.nga.giat:geowave-adapter-vector:jar:0.9.3: Failure to find mil.nga.giat:geowave-datastore-accumulo:jar:0.9.3 in http://download.osgeo.org/webdav/geotools/ was cached in the local repository, resolution will not be reattempted until the update interval of osgeo has elapsed or updates are forced -> [Help 1][ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.[ERROR] Re-run Maven using the -X switch to enable full debug logging.[ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles:[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/DependencyResolutionException[ERROR] [ERROR] After correcting the problems, you can resume the build with the command[ERROR] mvn -rf :rya.geoindexingchannel stoppedSetting status of a6c904c to FAILURE with url https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests-on-demand/46/ and message: 'Build finished. 'Using context: asfbot build |
|
asfbot build |
…added documentation to the project.
No description provided.