You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/09/20 19:13:56 UTC
[GitHub] [nifi] bbende opened a new pull request, #6435: NIFI-10528 Use nifi-standard-services-api-nar as parent of salesforce…
bbende opened a new pull request, #6435:
URL: https://github.com/apache/nifi/pull/6435
… NAR
<!-- Licensed to the Apache Software Foundation (ASF) under one or more -->
<!-- contributor license agreements. See the NOTICE file distributed with -->
<!-- this work for additional information regarding copyright ownership. -->
<!-- The ASF licenses this file to You under the Apache License, Version 2.0 -->
<!-- (the "License"); you may not use this file except in compliance with -->
<!-- the License. You may obtain a copy of the License at -->
<!-- http://www.apache.org/licenses/LICENSE-2.0 -->
<!-- Unless required by applicable law or agreed to in writing, software -->
<!-- distributed under the License is distributed on an "AS IS" BASIS, -->
<!-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -->
<!-- See the License for the specific language governing permissions and -->
<!-- limitations under the License. -->
# Summary
[NIFI-00000](https://issues.apache.org/jira/browse/NIFI-00000)
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [X] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [X] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- [X] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
### Pull Request Formatting
- [X] Pull Request based on current revision of the `main` branch
- [X] Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [X] Build completed using `mvn clean install -P contrib-check`
- [ ] JDK 8
- [X] JDK 11
- [ ] JDK 17
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] bbende commented on pull request #6435: NIFI-10528 Extrat JSON record readers to util module for use in Salesforce NAR
Posted by GitBox <gi...@apache.org>.
bbende commented on PR #6435:
URL: https://github.com/apache/nifi/pull/6435#issuecomment-1253897361
@tpalfy @Lehel44 I pushed an update that creates `nifi-json-record-utils` under `nifi-nifi-nar-bundles/nifi-record-utils` where we already have some similar patterns, and then update the Salesforce NAR and record serialization services NAR to depend on this.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] bbende commented on pull request #6435: NIFI-10528 Use nifi-standard-services-api-nar as parent of salesforce…
Posted by GitBox <gi...@apache.org>.
bbende commented on PR #6435:
URL: https://github.com/apache/nifi/pull/6435#issuecomment-1252863814
@Lehel44 ah thanks for pointing that out! I missed the use of `JsonTreeRowRecordReader` when making this change, and since I could only verify selecting a reader and oauth service and not actually run the processor, I suspect this change will cause a CNF exception at runtime.
Ideally I think we should strive for NAR dependencies to be used primarily for API NARs. Meaning for a processor to depend on a service API that it needs, or for implementing a service API, but not really for accessing implementation code in another NAR. Usually this can be achieved by factoring out some common code to a shared JAR that can be used in multiple NARs, but I'm not sure that works easily in this case.
A similar situation exists with `nifi-snowflake-nar` having a dependency on `nifi-dbcp-service-nar`, instead of depending on just the DBCP API from `nifi-standard-services-api-nar`. This was because `SnowflakeComputingConnectionPool extends DBCPConnectionPool` but I think it would be preferable to factor out some kind of shared `AbstractConnectionPool` that they both extend from.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] Lehel44 commented on pull request #6435: NIFI-10528 Use nifi-standard-services-api-nar as parent of salesforce…
Posted by GitBox <gi...@apache.org>.
Lehel44 commented on PR #6435:
URL: https://github.com/apache/nifi/pull/6435#issuecomment-1252821985
I remember that we used the nifi-record-serialization-services-nar in order to provide dependencies for the JsonTreeRowRecordReader. I'll take a look soon and mention @tpalfy too.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] bbende commented on pull request #6435: NIFI-10528 Extrat JSON record readers to util module for use in Salesforce NAR
Posted by GitBox <gi...@apache.org>.
bbende commented on PR #6435:
URL: https://github.com/apache/nifi/pull/6435#issuecomment-1253861530
I'll push an update shortly with a possible solution
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] asfgit closed pull request #6435: NIFI-10528 Extrat JSON record readers to util module for use in Salesforce NAR
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #6435: NIFI-10528 Extrat JSON record readers to util module for use in Salesforce NAR
URL: https://github.com/apache/nifi/pull/6435
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] tpalfy commented on pull request #6435: NIFI-10528 Extrat JSON record readers to util module for use in Salesforce NAR
Posted by GitBox <gi...@apache.org>.
tpalfy commented on PR #6435:
URL: https://github.com/apache/nifi/pull/6435#issuecomment-1256343815
LGTM
Thanks for the improvement @bbende!
Merged to main.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] bbende commented on pull request #6435: NIFI-10528 Use nifi-standard-services-api-nar as parent of salesforce…
Posted by GitBox <gi...@apache.org>.
bbende commented on PR #6435:
URL: https://github.com/apache/nifi/pull/6435#issuecomment-1252800071
@Lehel44 if you can take a look, thanks
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] bbende commented on pull request #6435: NIFI-10528 Use nifi-standard-services-api-nar as parent of salesforce…
Posted by GitBox <gi...@apache.org>.
bbende commented on PR #6435:
URL: https://github.com/apache/nifi/pull/6435#issuecomment-1253663725
@tpalfy If we include `nifi-record-serialization-services` jar in two different NARs, then services will be discovered twice.
In the current approach that exists in main, it is not discovered twice because the salersforce processors have:
```
<dependency>
<groupId>org.apache.nifi</groupId>
<artifactId>nifi-record-serialization-services</artifactId>
<version>1.18.0-SNAPSHOT</version>
<scope>provided</scope>
</dependency>
```
And then the salesforce NAR has a NAR dependency:
```
<dependency>
<groupId>org.apache.nifi</groupId>
<artifactId>nifi-record-serialization-services-nar</artifactId>
<version>1.18.0-SNAPSHOT</version>
<type>nar</type>
</dependency>
```
So at runtime it gets access to the class through this NAR dependency.
What I'm suggesting is that a processor shouldn't be directly instantiating an instance of a controller service (`JsonTreeRowRecordReader`). We should be trying to separate out whatever reusable logic we need from that service into utility or base classes that can then be used in both NARs.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] tpalfy commented on pull request #6435: NIFI-10528 Use nifi-standard-services-api-nar as parent of salesforce…
Posted by GitBox <gi...@apache.org>.
tpalfy commented on PR #6435:
URL: https://github.com/apache/nifi/pull/6435#issuecomment-1253586686
For `JsonTreeRowRecordReader` can't we just depend on the `nifi-record-serialization-services` _jar_ instead of the whole nar?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org