bitflicker64 opened a new pull request, #896:
URL: https://github.com/apache/hugegraph-toolchain/pull/896
## Purpose of the PR
No linked issue. These came out of a read through `hugegraph-loader`,
looking for small bugs and code that can move to Java 17 syntax without
changing behaviour.
Two of them change what a load does:
- `FileSource.asFileSource()` copies every read option except `time_zone`.
The failure-retry path (`LoadMapping.structsForFailure`) parses the failure
files through that copy, so dates in retried rows are read in the default
`GMT+8` zone, not the zone the mapping sets.
- `InputProgressDeser` has no case for `GRAPH`. A normal run drops empty
entries before it writes the progress file, but a load that is closed while
`loadStructs` is still running (the shutdown hook, or a call to
`HugeGraphLoader.shutdown()`) leaves the graph input in it. The next run with
`--incremental-mode true` then throws `AssertionError` while it reads that file.
## Main Changes
Bug fixes (each covered by a new test that fails without the fix):
- `FileSource.asFileSource()` copies `timeZone`.
- `InputProgressDeser` reads `GRAPH` progress the same way it reads `KAFKA`:
empty loaded and loading items.
- `RangesTimer` sorts ranges with
`Comparator.comparingLong(TimeRange::start)`. The old `(int) (o1.start() -
o2.start())` overflows when two starts are more than `Integer.MAX_VALUE` apart,
which sorts them in the wrong order.
- The invalid-boolean message in `DataTypeUtil.parseBoolean` printed the key
and the raw value in each other's places. It now reads `Failed to convert
'maybe'(key='key') to Boolean`.
Cleanup, no behaviour change:
- Pattern matching for `instanceof` where a cast of the same variable
follows the check: `DataTypeUtil`, `ElementBuilder`, `NopEdgeBuilder`,
`NopVertexBuilder`, `SinkToHBase`, `FileItemProgress`, `LoadProgress`,
`LoadUtil`, `MappingUtil`, `HugeGraphLoader`.
- `DataTypeUtil.convert`: drop the `source instanceof HDFSSource` check.
`HDFSSource` extends `FileSource`, so the `FileSource` check already covers it.
A new test parses dates through an `HDFSSource` to pin this.
- `HugeGraphLoader.loadStructs`: fold the `ParseException` and
`LoadException` branches into the `RuntimeException` branch (both are
`RuntimeException` subtypes and were rethrown unchanged), and remove a `catch
(Throwable t) { throw t; }`. The `finally` block runs in the same cases.
No CLI option, mapping or struct format, progress file format or config key
changes.
Left out on purpose: `KafkaSource.asFileSource()` copies only header,
charset and list format, so a Kafka failure retry also loses its date format,
time zone and delimiter. Which of those a retry should keep needs a decision on
intended behaviour, so it is not touched here.
## Verifying these changes
- [ ] Trivial rework / code cleanup without any test coverage. (No Need)
- [ ] Already covered by existing tests, such as *(please modify tests
here)*.
- [x] Need tests and can be verified as follows:
- `mvn -ntp -pl hugegraph-loader -am test -Dtest=UnitTestSuite
-Dsurefire.failIfNoSpecifiedTests=false` on JDK 17 (Linux): loader
`UnitTestSuite` 177 tests, 0 failures. The new `FileSourceTest` is registered
in the suite.
- With only `hugegraph-loader/src/main` reverted to master, 4 of the new
tests fail: `FileSourceTest.testAsFileSourceKeepsReadOptions`,
`LoadProgressTest.testParseGraphInputProgress`,
`DataTypeUtilTest.testConvertBoolean` and
`RangesTimerTest.testTimeRangeWithStartsFarApart`.
- `mvn test -P file` against a HugeGraph 1.8.0 server with auth on, set
up the way `start-hugegraph-servers.sh` does it: 99 tests, 96 passed. The two
`testHttps*ClientValueMapping` failures stop at `Start the shared Server
fixture or set hugegraph.test.trust-store-file`, because that run had no HTTPS
server, and `testParquetCompressFile` stops at `Failed to download file, please
check the network` before it loads anything.
## Does this PR potentially affect the following parts?
- [x] Nope
- [ ] Dependencies (add/update license info)
- [ ] Modify configurations
- [ ] The public API
- [ ] Other affects (typed here)
## Documentation Status
- [ ] `Doc - TODO`
- [ ] `Doc - Done`
- [x] `Doc - No Need`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
