[GitHub] [flink] becketqin commented on pull request #13512: [FLINK-19457][core] Add a number sequence generating source for the New Source API.

[GitBox <gi...@apache.org>]

becketqin commented on pull request #13512:
URL: https://github.com/apache/flink/pull/13512#issuecomment-702245190


   @StephanEwen Thanks for the explanation. I checked the classes and structure in `flink-core` again, I agree that this PR is in general following the existing convention. And I am fine with merge the PR as is. 
   
   That said, I had a hard time to fully understand the structure of `flink-core`. It would be great if you can help me understand this a bit more. I am wondering if the users would have the same questions like below.
   
   - I was initially expecting the function interfaces in `org.apache.api.java.functions` path, but there are only `KeySelector` and `NullByteKeySelector`. The actual function interfaces are in `org.apache.flink.common.functions` package. What is the difference between those two packages?
   - Also, I was not quite sure about the differences between `org.apache.flink.api.common.io` and `org.apache.flink.core.io`. 
   - In the existing package paths in `flink-core`, the implementation and interface classes are put in the same package. This PR introduces a new package `lib`. Should the other implementations also be put in a `lib` path?
   - In the existing package paths, `util` packages usually contains internal helper classes for the (abstract) implementations of the interfaces. This PR seems putting the abstract implementations of the `SourceReader` / `Enumerator` interfaces in the `util` package, and expecting other Iterator based sources to leverage that? Or are those classes also internal?
   - The follow up question to the above one, it seems that the `IteratorSourceReader`/ `IteratorSourceEnumerator` classes are just another base implementation of the `Source` interface, why the base implementation of the source in `flink-connector-base` was not put into `org.apache.flink.api.connector.source.lib.util`? 
   
   Sorry for sticking to discussion, my main concern is that If we ever want to revisit the API and package structure, right now might be the best timing, because we are essentially introducing new `Source` APIs and will probably deprecate old `SourceFunction` and `InputFormat` APIs in some coming release. This would be a natural migration towards new APIs if we want to do that. Keeping the API / package structure as is and changing them in the future could be even more disruptive.
   
   Thanks again.


----------------------------------------------------------------
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:
users@infra.apache.org