-
Notifications
You must be signed in to change notification settings - Fork 2.9k
NIFI-14064: Descriptions are not shown for Python processors #9565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
exceptionfactory
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@takraj Can you clarify and add some details about the changes in this pull request? The only runtime code change is the addition of Tags from Python Processors, but the title of the pull request says Descriptions are not shown. The description does appear to be set already in StandardRuntimeManifestService.createExtension().
The test updates appear to be useful, but given the scope of the changes, it would be helpful to revert the import ordering changes.
...py4j-integration-tests/src/test/java/org.apache.nifi.py4j/PythonControllerInteractionIT.java
Outdated
Show resolved
Hide resolved
|
@exceptionfactory Sure. The referenced PR #9288 changes the way component documentations are generated. From my understanding, earlier these documentations were somehow pre-generated during startup, but now it is loaded dynamically by the documentation view, using the NIFI REST API. Since this PR, when I add a Python processor, and open the doc view, basically only the name, version, and the relationships are shown. On the other hand, it works as expected for normal Java based processors. When I opened the browser's debug console I've spotted some error messages, when selecting any of the Python processors, that were referring to the missing 'tags' key from the API response. I have checked the endpoint manually, and compared the responses for Python and Java processors, and confirmed that some of the keys are missing in the former case. The Python processor had tags defined as well, but they were missing from the API response. And the rest is basically tracing back the code... |
d6304b2 to
a27a88b
Compare
exceptionfactory
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the additional background @takraj, that is helpful. For future clarity, summarizing the changes in the PR title and description, as opposed to just mentioning the behavioral bug, would be helpful.
Thanks for reverting the import ordering changes. I noted a few additional adjustments necessary regarding use of some collection classes, and otherwise this looks close to completion.
| import org.apache.nifi.python.PythonProcessorDetails; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.mockito.internal.util.collections.Sets; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Internal Mockito utility classes should never be used. It looks like this can be replaced with Set.of()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 030405d
...py4j-integration-tests/src/test/java/org.apache.nifi.py4j/PythonControllerInteractionIT.java
Outdated
Show resolved
Hide resolved
...ramework-core/src/test/java/org/apache/nifi/manifest/StandardRuntimeManifestServiceTest.java
Outdated
Show resolved
Hide resolved
...ramework-core/src/test/java/org/apache/nifi/manifest/StandardRuntimeManifestServiceTest.java
Outdated
Show resolved
Hide resolved
Bug's been present since fb2a82c / PR apache#9288
a27a88b to
030405d
Compare
|
@exceptionfactory I have updated the PR description to reflect my intent. |
exceptionfactory
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the adjustments and clarifications @takraj, looks good! +1 merging
- Set Tags in Extension definitions from Python Processor Details to avoid errors - Added and updated tests for manifests This closes apache#9565 Signed-off-by: David Handermann <exceptionfactory@apache.org> (cherry picked from commit 3c9ecd8)
Bug's been present since fb2a82c / PR #9288 , which changed the way documentations are generated. The new view dyamically fetches the information to be shown from the NIFI API, which did not return the component tags for Python processors in the manifest. Since the new documentation viewer always expects the tags to be returned, it has errored out when a python component was selected, which resulted in none of the documentation had been shown.
This PR attempts to fix this issue by adding the missing information to the API response.
Summary
NIFI-14064
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000Pull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
mvn clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation