dawidwys commented on a change in pull request #12201:
URL: https://github.com/apache/flink/pull/12201#discussion_r426407452
##########
File path:
flink-core/src/main/java/org/apache/flink/configuration/Configuration.java
##########
@@ -909,12 +909,10 @@ private void loggingFallback(FallbackKey fallbackKey,
ConfigOption<?> configOpti
List<String> listOfRawProperties =
StructuredOptionsSplitter.splitEscaped(o.toString(), ',');
return listOfRawProperties.stream()
.map(s ->
StructuredOptionsSplitter.splitEscaped(s, ':'))
- .map(pair -> {
+ .peek(pair -> {
Review comment:
Peek and map behave exactly the same in respect to the use cases
described in that thread. I'd say the thread discusses the declarative aspect
of the stream API.
In a code:
```
stream()
.map()/peek()
.count()
```
Neither `map` nor `peek` will be executed (depends on the jvm though) as
they cannot change the result of `count()`. The linked thread rather compares
`forEach` vs `peek`, in my opinion.
In our use case particularly, the purpose of the `peek` is to add a sanity
check right before the collection. We do not have any side effects in the
`peek` that we want applied to all results in the input collection. We want it
to be applied only to results we will collect. In this case in my opinion it is
absolutely safe to use `peek` and the code with `peek` is no different than
previous version with `map` (minus the return value).
Personally, I'd prefer to change it to `peek` as it removes IDE warning.
----------------------------------------------------------------
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]
