Skip to content

remove slf4j#7655

Closed
DaanHoogland wants to merge 1 commit intoapache:4.18from
shapeblue:remove_slf4j
Closed

remove slf4j#7655
DaanHoogland wants to merge 1 commit intoapache:4.18from
shapeblue:remove_slf4j

Conversation

@DaanHoogland
Copy link
Copy Markdown
Contributor

Description

This PR removes the references to slf4j in pom.xml, since it is no longer used.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

@DaanHoogland
Copy link
Copy Markdown
Contributor Author

@rohityadavcloud is this still needed for saml, maybe?

@DaanHoogland
Copy link
Copy Markdown
Contributor Author

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6294

@DaanHoogland
Copy link
Copy Markdown
Contributor Author

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link
Copy Markdown

[SF] Trillian test result (tid-6797)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 57639 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7655-t6797-kvm-centos7.zip
Smoke tests completed. 106 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_08_upgrade_kubernetes_ha_cluster Failure 3655.10 test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster Failure 0.08 test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown Error 308.12 test_kubernetes_clusters.py
test_01_migrate_VM_and_root_volume Error 81.85 test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks Error 53.41 test_vm_life_cycle.py

@DaanHoogland
Copy link
Copy Markdown
Contributor Author

@weizhouapache @borisstoyanov @vladimirpetrov I see the test_vm_life_cycle.py errors popping up on several PRs (i think only 4.18) is this explained by something environmental, or is should we look for a "guilty" merge?

Copy link
Copy Markdown
Member

@yadvr yadvr left a comment

Choose a reason for hiding this comment

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

LGTM based on changes, need regression/automated testing

@DaanHoogland
Copy link
Copy Markdown
Contributor Author

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6384

@DaanHoogland
Copy link
Copy Markdown
Contributor Author

@blueorangutan test matrix

@blueorangutan
Copy link
Copy Markdown

@DaanHoogland a [SF] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

@blueorangutan
Copy link
Copy Markdown

[SF] Trillian test result (tid-6943)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40650 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7655-t6943-kvm-centos7.zip
Smoke tests completed. 107 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_migrate_VM_and_root_volume Error 83.68 test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks Error 56.43 test_vm_life_cycle.py

@blueorangutan
Copy link
Copy Markdown

[SF] Trillian test result (tid-6941)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42176 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7655-t6941-xenserver-71.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@blueorangutan
Copy link
Copy Markdown

[SF] Trillian test result (tid-6942)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 44692 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7655-t6942-vmware-67u3.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@DaanHoogland DaanHoogland marked this pull request as ready for review July 25, 2023 09:19
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 25, 2023

Codecov Report

Merging #7655 (e7bd296) into 4.18 (f9b7bcf) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               4.18    #7655      +/-   ##
============================================
- Coverage     13.02%   13.01%   -0.01%     
- Complexity     9029     9030       +1     
============================================
  Files          2720     2720              
  Lines        256998   256998              
  Branches      40081    40081              
============================================
- Hits          33462    33439      -23     
- Misses       219336   219360      +24     
+ Partials       4200     4199       -1     

see 14 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Copy Markdown
Member

@vishesh92 vishesh92 left a comment

Choose a reason for hiding this comment

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

As per my understanding if slf4j is part of opensaml, it will start getting included in the jar after this change.
I also found this PR #6693 which introduced this change. Not sure if this is still valid anymore.

@weizhouapache weizhouapache removed this from the 4.18.1.0 milestone Aug 28, 2023
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