refactor: add common hugegraph output implement and writeType config #175
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
coderzc force-pushed the common_hg_output branch 2 times, most recently from 30f3d5a to 574eeb7 Compare 
coderzc force-pushed the common_hg_output branch from 574eeb7 to 9dff0aa Compare 
Codecov Report
@@ Coverage Diff @@ ## master #175 +/- ## ============================================ - Coverage 87.32% 87.18% -0.15% + Complexity 3058 3057 -1 ============================================ Files 324 325 +1 Lines 11465 11478 +13 Branches 995 995 ============================================ - Hits 10012 10007 -5 - Misses 983 1001 +18 Partials 470 470
Continue to review full report at Codecov.
|
coderzc changed the title coderzc changed the title 
javeme reviewed Dec 30, 2021
| ValueMinCombiner.class.getName()); | ||
| this.setIfAbsent(params, ComputerOptions.OUTPUT_CLASS, | ||
| LimitedLogOutput.class.getName()); | ||
| IdHugeGraphOutput.class.getName()); |
| BetweennessMessage.class.getName()); | ||
| this.setIfAbsent(params, ComputerOptions.OUTPUT_CLASS, | ||
| BetweennessCentralityOutput.class.getName()); | ||
| DoubleHugeGraphOutput.class.getName()); |
There was a problem hiding this comment.
prefer HugeGraphDoubleOutput
| TriangleCountValue.class.getName()); | ||
| this.setIfAbsent(params, ComputerOptions.OUTPUT_CLASS, | ||
| TriangleCountOutput.class.getName()); | ||
| IntHugeGraphOutput.class.getName()); |
| public static final ConfigOption<String> OUTPUT_RESULT_WRITE_TYPE = | ||
| new ConfigOption<>( | ||
| "output.result_write_type", | ||
| "The value is write-type of result output to hugegraph.", |
There was a problem hiding this comment.
The result write-type to output to hugegraph, allowed values are: [xx].
| return value; | ||
| } | ||
| | ||
| public WriteType writeType() { |
| .writeType(this.writeType()) | ||
| .ifNotExist() | ||
| .create(); | ||
| } |
There was a problem hiding this comment.
can we add a class HugeGraphDefaultOutput, and use switch-cache to create schema according to result value class option?
There was a problem hiding this comment.
it seems to make the code unclear
There was a problem hiding this comment.
yes, there are pros and cons.
pros: can use an unique output class instead of adding a new output class for each value type.
There was a problem hiding this comment.
moreover, this way is unable to process CustomizeValue
coderzc force-pushed the common_hg_output branch from f302422 to 84d328f Compare coderzc force-pushed the common_hg_output branch from 84d328f to 2988973 Compare javeme previously approved these changes Jan 11, 2022
| | ||
| public static class ClosenessWithWeightPropertyTestOutput | ||
| extends DoubleHugeGraphOutput { | ||
| extends HugeGraphDoubleOutput { |
corgiboygsj requested changes Jan 11, 2022
Comment on lines 46 to 47
| this.setIfAbsent(params, ComputerOptions.OUTPUT_RESULT_WRITE_TYPE, | ||
| WriteType.OLAP_RANGE.name()); |
There was a problem hiding this comment.
OLAP_COMMON is better, user can set OLAP_RANGE if they need
Comment on lines 45 to 46
| this.setIfAbsent(params, ComputerOptions.OUTPUT_RESULT_WRITE_TYPE, | ||
| WriteType.OLAP_RANGE.name()); |
Comment on lines 48 to 49
| this.setIfAbsent(params, ComputerOptions.OUTPUT_RESULT_WRITE_TYPE, | ||
| WriteType.OLAP_RANGE.name()); |
Comment on lines 47 to 48
| this.setIfAbsent(params, ComputerOptions.OUTPUT_RESULT_WRITE_TYPE, | ||
| WriteType.OLAP_RANGE.name()); |
Comment on lines 43 to 44
| this.setIfAbsent(params, ComputerOptions.OUTPUT_RESULT_WRITE_TYPE, | ||
| WriteType.OLAP_RANGE.name()); |
Comment on lines 133 to 134
| public static class BetweennessCentralityTestOutput | ||
| extends BetweennessCentralityOutput { | ||
| extends HugeGraphDoubleOutput { |
corgiboygsj approved these changes Jan 13, 2022
javeme approved these changes Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
No description provided.