MINIFICPP-2876 Manifest bundles invalid groupname - #2227
Conversation
|
I think copilot got overwhelmed by the 13k diff from the manifest change |
| #ifdef MINIFI_EXTENSION_GROUP_NAME | ||
| auto group_name = MAKESTRING(MINIFI_EXTENSION_GROUP_NAME); | ||
| #else | ||
| auto group_name = "org.apache.nifi.minifi"; | ||
| #endif |
There was a problem hiding this comment.
Can MINIFI_EXTENSION_GROUP_NAME be undefined? It may be better to force it to be defined, and generate a compile error otherwise.
| struct BundleCoordinate { | ||
| std::string name; | ||
| std::string group_name; | ||
| std::string version; | ||
|
|
||
| auto operator<=>(const BundleCoordinate& rhs) const = default; | ||
| }; |
There was a problem hiding this comment.
I think the previous name, BundleIdentifier, described the purpose of this struct better than BundleCoordinate. Can we revert to that?
There was a problem hiding this comment.
BundleCoordinate supposedly comes from nifi.
BundleCoordinate: https://github.com/apache/nifi/blob/main/nifi-framework-api/src/main/java/org/apache/nifi/bundle/BundleCoordinate.java
There is also a Bundle type in nifi-api: https://github.com/apache/nifi-api/blob/main/src/main/java/org/apache/nifi/flow/Bundle.java
I don't have a strong preference either way, because I don't know how well or how directly all of this maps to the corresponding NiFi types.
| auto& bundle_components = minifi::ClassDescriptionRegistry::getMutableClassDescriptions().try_emplace(details, details).first->second; | ||
| bundle_components.addClassDescription(description, ResourceType::Processor); |
There was a problem hiding this comment.
This try_emplace(details, details).first->second looks a bit confusing, and it is repeated many times. Can we add a helper function so that we can write something like
| auto& bundle_components = minifi::ClassDescriptionRegistry::getMutableClassDescriptions().try_emplace(details, details).first->second; | |
| bundle_components.addClassDescription(description, ResourceType::Processor); | |
| auto& bundle_components = minifi::ClassDescriptionRegistry::getMutableComponents(bundle_identifier); | |
| bundle_components.addClassDescription(description, ResourceType::Processor); |
instead?
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically main)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.