cameronlee314 opened a new pull request #1352: URL: https://github.com/apache/samza/pull/1352
Issues: 1. If using Scala 2.11, there is a compile error when trying to use a Java class extend a Scala trait with implemented methods. BaseKeyValueStorageEngineFactory can be considered part of the Samza API, but it is a Scala trait with an implemented method, so that restricts extension. 2. It would be good to migrate away from Scala code in general, for consistency with other new Samza code. Changes: 1. `BaseKeyValueStorageEngineFactory` is now an abstract class written in Java. 2. Added some unit tests for `BaseKeyValueStorageEngineFactory`. Tests: 1. Added unit tests 2. Some tests in samza-test use in-memory key-value storage, and those tests pass. 3. Deployed job with "drop.large.messages" enabled and verified that messages get dropped 4. Deployed job with "disallow.large.messages" enabled and verified that exception is thrown 5. Deployed job with changelog enabled and verified that changelog was being written to API changes: `BaseKeyValueStorageEngineFactory` is no longer a Scala trait, so that could potentially impact existing inheritors: 1. Any existing Scala classes which uses the `with` syntax to mix in `BaseKeyValueStorageEngineFactory` will need to be updated to use `BaseKeyValueStorageEngineFactory` as an abstract class instead. If an existing Scala class used `extends BaseKeyValueStorageEngineFactory` (such as how `InMemoryKeyValueStorageEngineFactory` and `RocksDbKeyValueStorageEngineFactory` uses `BaseKeyValueStorageEngineFactory`), then it should not need to change. 2. Scala 2.12 compiles traits into interfaces with default methods, so a Java class built against the Scala 2.12 version of `BaseKeyValueStorageEngineFactory` won't be able to use it as an interface. I suppose `BaseKeyValueStorageEngineFactory` could work as an interface with a default implementation for `getStorageEngine`, but it seems like abstract class fits the pattern better, since `getKVStore` shouldn't be a public method. Upgrade instructions: Any custom implementation of `BaseKeyValueStorageEngineFactory` needs to treat it as an abstract class, not a trait/interface. If it is necessary to make changes, only class inheritance structure should need to be changed. The functionality/signature of `getStorageEngine` and the usage of the `getKVStore` abstract method are the same, so those pieces should stay the same. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
