[jira] [Commented] (FLINK-16274) Add typed builder methods for setting dynamic configuration on StatefulFunctionsAppContainers

["Igal Shilman (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FLINK-16274?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17045574#comment-17045574 ] 

Igal Shilman commented on FLINK-16274:
--------------------------------------

Hey [~tzulitai],
as it indeed looks nicer in the code,  I have to concerns:
a) Having users (statefun core devs) supplying the flink-conf.yaml as part of the e2e test makes the e2e test
more likely to catch property renaming in Flink.
b) It requires pulling in a dependency on flink-statefun-core into the test driving code, that is otherwise independent of the runtime which is a very nice property for an end-to-end black box test.
Although I don't see immediately a disadvantage to adding this dependency (since the actual system under test executing within a container with a different classpath) I wouldn't want to give this property for the reason described in the PR.

Having said all that, since I haven't wrote an e2e test with the new abstraction yet, I wasn't feeling the pain,
so if you think the points mentioned above are worth trading for that convince method then lets do that (especially that the work was already done)

> Add typed builder methods for setting dynamic configuration on StatefulFunctionsAppContainers
> ---------------------------------------------------------------------------------------------
>
>                 Key: FLINK-16274
>                 URL: https://issues.apache.org/jira/browse/FLINK-16274
>             Project: Flink
>          Issue Type: New Feature
>          Components: Stateful Functions, Test Infrastructure
>    Affects Versions: statefun-1.1
>            Reporter: Tzu-Li (Gordon) Tai
>            Assignee: Tzu-Li (Gordon) Tai
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> Excerpt from: https://github.com/apache/flink-statefun/pull/32#discussion_r383644382
> Currently, you'd need to provide a complete {{Configuration}} as dynamic properties when constructing a {{StatefulFunctionsAppContainers}}.
> It'll be nicer if this is built like this:
> {code}
> public StatefulFunctionsAppContainers verificationApp =
>     new StatefulFunctionsAppContainers("sanity-verification", 2)
>         .withModuleGlobalConfiguration("kafka-broker", kafka.getBootstrapServers())
>         .withConfiguration(ConfigOption option, configValue)
> {code}
> And by default the {{StatefulFunctionsAppContainers}} just only has the configs in the base template {{flink-conf.yaml}}.
> This would require lazy construction of the containers on {{beforeTest}}.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)