Skip to content

Hop server enhancement #4569#4574

Closed
nadment wants to merge 1 commit into
apache:mainfrom
nadment:4569
Closed

Hop server enhancement #4569#4574
nadment wants to merge 1 commit into
apache:mainfrom
nadment:4569

Conversation

@nadment

@nadment nadment commented Nov 15, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • add command line options for (host, port, shutdownport, proxyhost, proxyport,ssl)
  • support for override properties on the command line
  • add shutdown listener to web server
  • add shutdown servlet
  • add RemoteHopServer class to interact with a Hop server and clean HopServerMeta
  • remove setup hop server syntax by hostname and port
  • add HopServerTerminate extension point after shutdown
  • all HopServerXXX extension points now provide a HopServer object, rather than a HopServer/WebServer mix
  • add command line options --help and --version
  • set abbreviateSynopsis and usageHelpAutoWidth to command line

Please add a meaningful description for your change here

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Run mvn clean install apache-rat:check to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
  • If you have a group of commits related to the same change, please squash your commits into one and force push your branch using git rebase -i.
  • Mention the appropriate issue in your description (for example: addresses #123), if applicable.

To make clear that you license your contribution under the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

@nadment nadment linked an issue Nov 15, 2024 that may be closed by this pull request
[Feature Request]: Hop server enhancement #4569
Closed
@nadment nadment force-pushed the 4569 branch 3 times, most recently from 6237f6c to 6a3ce3c Compare November 15, 2024 23:47
@nadment
Hop server enhancement apache#4569
63c0557 
- add command line options for (host, port, shutdownport, proxyhost,
proxyport,ssl)
- support for override properties on the command line
- add shutdown listener to web server
- add shutdown servlet
- add RemoteHopServer class to interact with a Hop server and clean
HopServerMeta
- remove setup hop server syntax by hostname and port
- add HopServerTerminate extension point after shutdown
- all HopServerXXX extension points now provide a HopServer object,
rather than a HopServer/WebServer mix
- Set abbreviateSynopsis and usageHelpAutoWidth to command line
@bamaer

bamaer commented Nov 24, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

we need better option grouping, port appears as sub-option of password, host appears as sub-option of help etc:

  -h, --help              Displays this help message and quits.
      --host=<host>       The server host name or IP address to use.

  -p, --password=<password>
                          The server password. Required for administrative operations only, not for starting the server.
      --port=<port>       The server port to use.

@bamaer

bamaer commented Nov 24, 2024

Copy link
Copy Markdown
Contributor

tested:

  • additional options for host, port, username, password,
  • -v and -h options
  • shutdown/kill only works when correct authentication is provided
  • --ssl options doesn't seem to work

@nadment

nadment commented Nov 24, 2024

Copy link
Copy Markdown
Contributor Author

For option grouping, this is the behavior of Picoli if we do not define a unique letter for an option.

@hansva

hansva commented Jan 23, 2025

Copy link
Copy Markdown
Contributor

is this still stuck in draft for an actual reason? @nadment

@nadment

nadment commented Jan 23, 2025

Copy link
Copy Markdown
Contributor Author

No for me PR is ok. It's a lot of changes to test. I haven't found a better solution to group Picoli options

@hansva

hansva commented Jan 23, 2025

Copy link
Copy Markdown
Contributor

Alright, then I'll start testing and reviewing this. There are some other minor changes (deprecations) that I will probably include in here.

@nadment nadment marked this pull request as ready for review January 23, 2025 17:28
@hansva

hansva commented Oct 26, 2025

Copy link
Copy Markdown
Contributor

let's try and pick this one up after the release again :)

@mattcasters

mattcasters commented Dec 15, 2025

Copy link
Copy Markdown
Contributor

I think it's printed this way because Picocli tries to align short format options and long format ones. In this case it's probably not finding a short hand option for --port.

@hansva

hansva commented Jan 10, 2026

Copy link
Copy Markdown
Contributor

@nadment, what are we going to do with this one? Will you resolve conflicts so we can merge it?

@nadment

nadment commented Jan 10, 2026

Copy link
Copy Markdown
Contributor Author

Okay, I'll take care of that old pull request soon.

@nadment nadment mentioned this pull request Jan 20, 2026
Closed
@mattcasters mattcasters mentioned this pull request Jan 23, 2026
Closed
@hansva hansva mentioned this pull request Jul 10, 2026
Merged
5 tasks
@hansva

hansva commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

re-applied with #7479

@hansva hansva closed this Jul 10, 2026
hansva added a commit that referenced this pull request Jul 10, 2026
@hansva @nadment @claude
Hop server enhancement #4569 (rebased onto current main) (#7479)
893e59e 
Reapplies #4574 (Nicolas Adment) on top of current main, which
had diverged ~1720 commits (incl. the Apache HttpClient 4->5 migration).

- Split HopServerMeta into a slim metadata class + a new RemoteHopServer
  client that holds all remote-interaction methods. RemoteHopServer is
  rebuilt from main's current (HttpClient 5) method bodies, not the PR's
  Nov-2024 versions, so no server fixes are lost.
- All callers use the new RemoteHopServer request* API (RemotePipelineEngine,
  RemoteWorkflowEngine, RemoteExecutionInfoLocation, GetServerStatus,
  HopGuiWorkflowRunDelegate, HopServerMetaEditor, HopServer).
- Graceful shutdown: new ShutdownServlet (GET /shutdown, authenticated) +
  HopServer.shutdown() + HopServerTerminate extension point + a JVM shutdown
  hook so SIGTERM/CTRL-C stop the server gracefully. WebServer gets idempotent
  start()/stop() and drops the old WebServerShutdownHook /
  IWebServerShutdownHandler / MonitorThread. HopServerStartup/Shutdown/Terminate
  now all receive a HopServer object.
- WebServer.CONST_PORT renamed to DEFAULT_PORT.
- Removed the vestigial shutdownPort end to end (HopServerMeta metadata field,
  WebServer/HopServer/HopServerConfig plumbing, the hostname:port:shutdownport
  CLI positional, sample hop-server.xml, docker env/EXPOSE, docs). It was never
  wired to a listener; graceful shutdown is covered by SIGTERM and /shutdown.
- Removed the dead --kill example + callStopHopServerRestService machinery.

Reconciled with main's evolution: kept main's picocli CLI (it already has
--help/--version and still supports the hostname:port syntax); ShutdownServlet
is on the main connector for Jetty ee11 (no separate shutdown-port connector);
dropped the dead propertiesMasterName fields main removed in #5060; docs updated
(Stopping Hop Server section + shutdown-port references).

Co-authored-by: Nicolas Adment <39568358+nadment@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Hop Gui Hop Server

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Feature Request]: Hop server enhancement

4 participants

@nadment @bamaer @hansva @mattcasters