aalexandrov edited a comment on pull request #13770: URL: https://github.com/apache/flink/pull/13770#issuecomment-721757666
Thanks for the thorough review @twalthr. I addressed most of the individual comments (additional feedback is required where the comment is not marked as *resolved*). Regarding your high-level comments: ### Testing I have a working end-to-end test written as an example in `flink-examples-table`. I can rewrite it as an `~ITCase`, however it is not clear to me what kind of workflow I should use to create / delete the backing Kinesis streams. Do we have an AWS account that we can use for ITCases? If not, I suggest to add a working `~ITCase` as `@ignored` and enable it when we decide how to resolve the issue of resource provisioning and access management for the AWS resources required for the integration tests. ### Property naming I will implement the suggestion proposed [at the bottom of this comment](https://github.com/apache/flink/pull/13770#discussion_r517349077). I will try to fix that today. ### Partitioning I was following the advise from the `SupportsPartitioning` JavaDoc: > If only a subset of all partition keys (a prefix part) get a value assigned in the PARTITION clause, the operation is considered as an "insertion into a dynamic partition". The term *prefix part* is quite confusing in this case, a better naming is *static part* or *fixed part*. I will try to fix that today. ---------------------------------------------------------------- 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]
