Lehel44 commented on code in PR #7588:
URL: https://github.com/apache/nifi/pull/7588#discussion_r1299779123
##########
nifi-nar-bundles/nifi-hadoop-bundle/nifi-hdfs-processors/src/main/java/org/apache/nifi/flow/resource/hadoop/HDFSExternalResourceProvider.java:
##########
@@ -140,13 +146,16 @@ public InputStream fetchExternalResource(final
ExternalResourceDescriptor descri
final HdfsResources hdfsResources = getHdfsResources();
try {
- return
hdfsResources.getUserGroupInformation().doAs((PrivilegedExceptionAction<FSDataInputStream>)
() -> {
- if (!hdfsResources.getFileSystem().exists(path)) {
- throw new IOException("Cannot find file in HDFS at
location " + location);
- }
-
- return hdfsResources.getFileSystem().open(path,
BUFFER_SIZE_DEFAULT);
- });
+ final FSDataInputStream fsDataInputStream =
+
hdfsResources.getUserGroupInformation().doAs((PrivilegedExceptionAction<FSDataInputStream>)
() -> {
+ if (!hdfsResources.getFileSystem().exists(path)) {
+ throw new IOException("Cannot find file in HDFS at
location " + location);
+ }
+
+ return hdfsResources.getFileSystem().open(path,
BUFFER_SIZE_DEFAULT);
+ });
+ // The acquired InputStream is used by the client and cannot be
closed here. The decorator is responsible for that.
Review Comment:
Maintaining code readability and understandability is crucial for the
long-term maintainability of a project. While it's true that using well-known
design patterns like the decorator pattern can help convey the underlying
structure of the code to experienced developers, it's important to consider the
diverse range of developers who might interact with the codebase. By providing
clear and concise comments that explain the 'why' behind design decisions, even
those less familiar with certain patterns can grasp the purpose and
functionality of a particular piece of code. These comments serve as a form of
documentation, helping new team members onboard quickly and aiding in debugging
or modification scenarios. In complex scenarios like decorator patterns, where
multiple classes are interacting to achieve a specific goal, comments can be an
invaluable guide for developers who are trying to make sense of the code. In
essence, comments that elucidate the rationale behind design choices en
able better collaboration and reduce the risk of misinterpretation or
unintended modifications.
While the design pattern might provide a concise description, it remains
valuable to explicitly highlight the role of the **HDFSResourceInputStream as
the decorator**. Additionally, it's worth clarifying that **it's the FileSystem
which must remain open until the acquired InputStream is fully processed,
rather than the InputStream itself**. The decorator takes on the responsibility
of closing the FileSystem when the acquired InputStream is eventually closed.
This specific closing logic is a customized implementation that stands apart
from the general pattern description.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
