NIFI-12201 - Deprecation marking for nifi-toolkit-tls in NiFi 1.x

[greyp9]

Summary

NIFI-00000

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

• Apache NiFi Jira issue created

Pull Request Tracking

• Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
• Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

• Pull Request based on current revision of the main branch
• 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

• Build completed using mvn clean install -P contrib-check
  • JDK 21

Licensing

• New dependencies are compatible with the Apache License 2.0 according to the License Policy
• New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

• Documentation formatting appears as expected in rendered files

[ChrisSamo632]

Thanks for the update @greyp9, can we use the Deprecation Logger instead of System.out for noting the pending removal of this utility?

[exceptionfactory]

Thanks for introducing this message @greyp9, this approach looks good under the circumstances.

@ChrisSamo632 For other components, the DeprecationLogger and the Deprecated annotation make sense, but it does seem necessary in this particular scenario. The TlsToolkitMain is a command class that gets called through wrapping scripts, as opposed to a class that other components use. From that perspective, using DeprecationLogger does not add much, and it requires a configured logger implementation. For the same reasons, the Deprecated annotation does not provide a warning for end users of the TLS Toolkit, as it is developer-facing.

With that background, this approach seems sufficient to me. What do you think?

[ChrisSamo632]

That's probably fair enough, thanks for the explanation @exceptionfactory

I wonder whether this should be a System.err call instead though, to prevent problems for anyone capturing and parsing STDOUT currently for this utility. But I'm not going to push hard for such a change

[greyp9]

That's probably fair enough, thanks for the explanation @exceptionfactory

I wonder whether this should be a System.err call instead though, to prevent problems for anyone capturing and parsing STDOUT currently for this utility. But I'm not going to push hard for such a change

Thanks much for the input. I'm switching it over to System.err.

[ChrisSamo632]

Thanks for the update, LGTM 👍

Will merge tomorrow (if I'm not beaten to it), when hopefully the build failures will have been fixed on the support branch (unrelated to this change)

[exceptionfactory]

Thanks for making the adjustments @greyp9, and thanks for the review @ChrisSamo632! The build failures are unrelated, and now resolved in a recent commit. +1 merging

[ChrisSamo632]

This has been merged to support/nifi-1.x