Skip to content
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

Remove Audit Rest API invokation #27692

Merged
merged 1 commit into from
May 24, 2024
Merged

Conversation

wrodrig
Copy link
Contributor

@wrodrig wrodrig commented Feb 20, 2024

As it is, when the audit-1.0 feature is enabled, it automatically pulls rest-handler dependecies.

When this new audit feature is enabled, the rest-handler dependecies will only be added when is enabling features that enable restHandler-1.0 functionalities.

This PR is looking to address #27475

@wrodrig wrodrig self-assigned this Feb 20, 2024
Copy link

@helyarp helyarp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks okay. Maybe update the last copyr year.

@posmith
Copy link

posmith commented Feb 28, 2024

Support message review not required.

@wrodrig wrodrig force-pushed the audit-rest branch 4 times, most recently from a361616 to 4b1ea4a Compare April 15, 2024 23:05
@wrodrig wrodrig requested a review from jhanders34 April 23, 2024 16:47
Copy link
Member

@jhanders34 jhanders34 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't update every file that had the same issues, so if you see a comment, you may want to also look at the other files to see if the problem is there as well. Additional things to look at / fix:

  1. Update copyright date in files
  2. Update your IDE to use 4 spaces for indentation instead of tabs. It really makes it hard to read the code when you have 8 character tabs.
  3. Instead of repeating the FATSuite with audit-2.0, it will be better to do it for the individual test especially when only 1 or 2 tests in a FATSuite actually test with audit. Doing the repeat has no value on those tests that aren't using the audit-1.0 feature.
  4. I don't think it is right to add the restConnector feature to the list of features when switching from audit 1.0 to 2.0. I think you should just add audit-2.0 in the FeatureReplacementAction.
  5. I think it is overkill to add in the audit common.tooling for doing the FeatureReplacementAction. In light of #4 it is just audit-1.0 to audit-2.0. No reason to bring in the common.tooling for constants to be used for the FeatureReplacementAction.

@wrodrig wrodrig force-pushed the audit-rest branch 9 times, most recently from f0de8d7 to 2ba2740 Compare April 29, 2024 19:45
@wrodrig wrodrig requested review from jhanders34, utle and helyarp April 29, 2024 19:46
Copy link

@helyarp helyarp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@wrodrig
Copy link
Contributor Author

wrodrig commented Apr 29, 2024

#build

@LibbyBot
Copy link

Please code review feature-related files, @OpenLiberty/delivery-approvers

@LibbyBot
Copy link

@LibbyBot
Copy link

The build wrodrig-27692-20240520-1730
https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_dM4k0BbFEe-aC7a28EbkCw
completed and has errors or failures.

For help analyzing your personal build, go to https://libh-proxy1.fyre.ibm.com/cognitive/buildAnalysis.html?uuid=_dM4k0BbFEe-aC7a28EbkCw

@wrodrig
Copy link
Contributor Author

wrodrig commented May 21, 2024

#build
#libby

@LibbyBot
Copy link

Your personal pipeline request is at https://libh-proxy1.fyre.ibm.com/cognitive/pipelineAnalysis.html?uuid=2d75f6ac-8af4-4aa3-8565-d7f3610b64fc

Target locations of links might be accessible only to IBM employees.

@LibbyBot
Copy link

Please code review feature-related files, @OpenLiberty/delivery-approvers

@LibbyBot
Copy link

Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_vtck0BeoEe-aC7a28EbkCw

Target locations of links might be accessible only to IBM employees.

@LibbyBot
Copy link

The build wrodrig-27692-20240521-0436
https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_BqR94BblEe-aC7a28EbkCw
completed and has errors or failures.

For help analyzing your personal build, go to https://libh-proxy1.fyre.ibm.com/cognitive/buildAnalysis.html?uuid=_BqR94BblEe-aC7a28EbkCw

@LibbyBot
Copy link

@LibbyBot
Copy link

@wrodrig
Copy link
Contributor Author

wrodrig commented May 22, 2024

#build

@@ -282,6 +282,8 @@ private static Set<String> getExtendedCompatibleFeatures(Set<String> compatibleF
// remove logAnalysis-1.0. It depends on hpel being configured
features.remove("logAnalysis-1.0");

feature.remove("audit-2.0");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
feature.remove("audit-2.0");
features.remove("audit-2.0");

This is what caused your build break.

@LibbyBot
Copy link

Please code review feature-related files, @OpenLiberty/delivery-approvers

@LibbyBot
Copy link

Your personal pipeline request is at https://libh-proxy1.fyre.ibm.com/cognitive/pipelineAnalysis.html?uuid=ab2a6cf1-d52a-45f7-988d-5c607f570cbd

Target locations of links might be accessible only to IBM employees.

@LibbyBot
Copy link

Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_KAr1wBhyEe-aC7a28EbkCw

Target locations of links might be accessible only to IBM employees.

jhanders34
jhanders34 previously approved these changes May 22, 2024
@LibbyBot
Copy link

@jhanders34
Copy link
Member

#libby

@LibbyBot
Copy link

The build wrodrig-27692-20240522-1341
https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_KAr1wBhyEe-aC7a28EbkCw
completed successfully!

    In this pull request we are working to add the `audit-2.0` feature. This feature is very similar to audit-1.0 feature the main difference is that it doesn't involve rest handler features. More information about this feature can be found here:

    OpenLiberty#27475
@jhanders34
Copy link
Member

#libby

@LibbyBot
Copy link

Code analysis and actions

DO NOT DELETE THIS COMMENT.
  • 31 product code files were changed.

  • Please describe in a separate comment how you tested your changes.

  • 5 infrastructure code files were changed.

  • 2 test infrastructure code files were changed.

  • Test failures/errors in the build could be due to these changes.

  • 29 FAT files were changed, added, or removed.

  • Check that the build did not break the affected FAT suite(s).

  • 1 messages files were changed and need an L2 review.

  • @OpenLiberty/message-reviewer Please review.

  • dev/com.ibm.ws.request.probe.audit.servlet/resources/com/ibm/ws/request/probe/audit/servlet/internal/resources/ProbeAuditServletMessages.nlsprops

  • 16 NLS files were changed and need an ID review.

  • @OpenLiberty/message-reviewer Please review.

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_es.properties

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_ko.properties

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_ru.properties

  • dev/com.ibm.ws.request.probe.audit.servlet/resources/com/ibm/ws/request/probe/audit/servlet/internal/resources/ProbeAuditServletMessages.nlsprops

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_cs.properties

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_zh.properties

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_pt_BR.properties

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0.properties

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_zh_TW.properties

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_it.properties

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_ja.properties

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_ro.properties

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_pl.properties

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_hu.properties

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_de.properties

  • dev/com.ibm.websphere.appserver.features/visibility/public/audit-2.0/resources/l10n/com.ibm.websphere.appserver.audit-2.0_fr.properties

@wrodrig wrodrig merged commit c3c1a6e into OpenLiberty:integration May 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants