Skip to content

NMS-20165: remove beanshell - #8744

Open
dino2gnt wants to merge 5 commits into
developfrom
dcy/no-beanshell
Open

NMS-20165: remove beanshell#8744
dino2gnt wants to merge 5 commits into
developfrom
dcy/no-beanshell

Conversation

@dino2gnt

@dino2gnt dino2gnt commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Will update these groovy sites to JSR-223 in a future PR.

External References

@christianpape christianpape left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So, we do not support any use of bsh.util.BeanShellBSFEngine anymore, right? This means the docs also need to be updated: BSFMonitor.adoc, BsfDetector.adoc and scriptd.adoc.

  BSFMonitor.adoc (-52 lines net)
  - Migration note near the top: Groovy is what ships, BeanShell is gone, rewrite .bsh scripts and update bsf-engine/lang-class.
  - BeanShell dropped from the bsf-engine parameter list, the lang-class description, and the language-settings table.
  - Entire "BeanShell example" section removed.
  - Corrected a now-wrong install instruction: the Groovy section told users to "copy a compatible groovy-all.jar into $OPENNMS_HOME/lib and restart." Groovy ships with OpenNMS, and groovy-all hasn't been the right artifact since Groovy 2.5 split into modules. Now reads "ships with {page-component-title}, so no additional library
  is required."
  - Jython kept as-is; BSF still supports it and it genuinely does need a user-supplied jar.

  BsfDetector.adoc (-43 lines net)
  - Same migration note, adapted to the detector's bsfEngine/langClass parameter names.
  - BeanShell removed from both parameter choice lists and the language table; "BeanShell example" section removed.
  - The IMPORTANT about placing a library in lib now applies to Jython only, since it was previously scoped to "Groovy or Jython."

  Two unrelated defects I fixed while in the files

  - Three Groovy blocks in BSFMonitor.adoc were tagged [source, java], so they rendered with Java highlighting. Now [source, groovy].
  - The generic response-time-tracking snippet used rrd-base-name of minimalbshbase — a BeanShell-flavored name in a language-neutral section. Renamed to myscriptbase.

  Verification

  All five Groovy blocks across the two pages extracted, callout markers stripped, and compiled with GroovyShell.parse against a classpath including both opennms-services and features/poller/monitors/core — all pass, so even the advanced example's reference to org.opennms.netmgt.poller.monitors.BSFMonitor resolves. Both pages
  render through asciidoctor with exit 0 and no warnings, which also confirms the 4-column tables survived row removal.
@github-actions github-actions Bot added the docs label Aug 7, 2026
@dino2gnt
dino2gnt requested a review from christianpape August 11, 2026 13:24
christianpape
christianpape previously approved these changes Aug 11, 2026

@christianpape christianpape left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

@dino2gnt

Copy link
Copy Markdown
Contributor Author

Had to resolve a merge conflict.

@dino2gnt
dino2gnt requested a review from christianpape August 13, 2026 13:33
christianpape
christianpape previously approved these changes Aug 13, 2026

@christianpape christianpape left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

@marshallmassengill marshallmassengill left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@dino2gnt

Copy link
Copy Markdown
Contributor Author

Somehow introduced stray words in the dependencies/spring-security/pom.xml. Fixed.

@marshallmassengill marshallmassengill left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

christianpape
christianpape previously approved these changes Aug 13, 2026

@christianpape christianpape left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

@dino2gnt

Copy link
Copy Markdown
Contributor Author

Fixed the integration tests for BSFNotificationStrategy after removing Beanshell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants