ThePrez / ServiceCommander-IBMi

Service Commander for IBM i
Apache License 2.0
40 stars 12 forks source link

[FEATURE-REQUEST] Make *SC TCPSVR use the joblog only - like other TCP/IP servers #186

Closed chrjorgensen closed 1 year ago

chrjorgensen commented 1 year ago

Describe the solution you'd like

Currently the TCPSVR start and stop commands STRTCPSVR and ENDTCPSVR writes all output from the PASE sc command to the joblog - and also to the stdout. The write to stdout makes the output immediately visible for the user - but it is also a problem when ending the system using GO SAVE 21 (and probably other shutdown methods): The system is ending all TCP/IP servers including SC, and after ending SC the job is waiting on the console for the user to press Enter - thus halting the shutdown.

IMHO it would be better for the SC TCPSVR to act like other TCP/IP servers and only write the output to the joblog. The system will always write a message to joblog indicating whether the start/stop succeeded or failed, so the user will still be able to determine the success of the operation and can get more info by viewing the joblog. This change would allow GO SAVE 21 to continue when SC is ended and not halt with display of stdout.

The current behavior could be based on an environment variable (e.g. SC_TCPSVR_OUTPUT=DISPLAY) so any user accustomed to the current behavior can continue having this setup.

WDYT?

Describe alternatives you've considered

Additional context

ThePrez commented 1 year ago

Does the existing batch support accomplish this ("SC_TCPSVR_SUBMIT")?

Since the sc output now gets routed to the job log, even when not running in batch, it may be ok to change the default behavior to avoid stdout/stderr entirely. Anyone accustomed to the current behavior would just wait a minute and see the output in the job log (of course, that assumes they are running in QCMD or similar). Definitely worth considering though...

chrjorgensen commented 1 year ago

Actually, the SC_TCPSVR_SUBMIT environment variable will circumvent the problem, since it will submit the end of all SC services to jobqueue QUSRNOMAX. I didn't think of that...

However, the submit variable was mainly introduced to fix the problem of two simultaneously running STRTCPSVR commands - not stop the output from stdout. IMO it should be sufficient to have the output from ENDTCPSVR SC and STRTCPSVR SC as entries in the joblog - this is where other TCP/IP servers send their output and where IBM i users normally look for events and information in the job. If *SC is the only server started or stopped, there will be a message telling the result of the operation.

If this is a too radical change, then we could add the environment variable as proposed to keep the current behavior. Anyone not having this will have a normal GO SAVE 21 and ENDTCPSVR *ALL and ENDTCP ENDSVR(*YES).

ThePrez commented 1 year ago

I think I'd propose simply removing this printf: https://github.com/ThePrez/ServiceCommander-IBMi/blob/51cce1dbab3f680eb495cf284e8729447c2fbcef/strtcpsvr/sc_tcpsvr.c#L347

The "real-time" feedback seemed like a nicety but it also makes this handler behave different from normal STRTCPSVR handlers in the cases you've found.

Given your statement

IMO it should be sufficient to have the output from ENDTCPSVR SC and STRTCPSVR SC as entries in the joblog - this is where other TCP/IP servers send their output and where IBM i users normally look for events and information in the job. If *SC is the only server started or stopped, there will be a message telling the result of the operation.

I think you agree with this change, but please confirm.

chrjorgensen commented 1 year ago

@ThePrez I confirm I agree... :-)

chrjorgensen commented 1 year ago

@ThePrez I will give it a shot - PR coming soon...

ThePrez commented 1 year ago

Addressed in #188. Closing