[tools][server] Environment variables for sc-server host and port #453
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #453 +/- ##
==========================================
- Coverage 95.70% 95.69% -0.02%
==========================================
Files 237 237
Lines 26965 26971 +6
Branches 1863 1865 +2
==========================================
+ Hits 25808 25809 +1
- Misses 1157 1162 +5 |
|
@ellipsis review this |
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 768edc8 in 2 minutes and 5 seconds
More details
- Looked at
115lines of code in6files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. sc-tools/sc-server/src/sc_server_module.cpp:27
- Draft comment:
Usenullptrinstead ofnull_ptrfor checking null pointers.
if (port != nullptr)
- Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_aFcRgB3hjL7iyJRw
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
…nment variables for sc-server host and port );
This comment was marked as duplicate.
This comment was marked as duplicate.
…nment variables for sc-server host and port );
|
@NikitaZotov, I have addressed your comments in pull request #455 You can configure Ellipsis to address comments with a direct commit or a side PR, see docs. @NikitaZotov: @ellipsis fix this |
…nment variables for sc-server host and port );
…nment variables for sc-server host and port );
| @@ -21,7 +21,7 @@ ScServer::ScServer(std::string hostName, size_t port) | |||
|
|
|||
| { | |||
| LogMessage(ScServerErrorLevel::info, "Socket data:"); | |||
| LogMessage(ScServerErrorLevel::info, "\tHost name: " + m_hostName); | |||
| LogMessage(ScServerErrorLevel::info, "\tHost: " + m_hostName); | |||
There was a problem hiding this comment.
if message is changed to "Host", shouldn't m_hostName be changed to m_host as well?
| // removed. | ||
| // TODO(NikitaZotov): Configure all platform-dependent components from kb. | ||
|
|
||
| sc_char const * host = std::getenv("SC_SERVER_HOST"); |
There was a problem hiding this comment.
can this be tested in some way?
Now there is no problem to specify host and port for sc-server in docker-compose.yml in projects.
Important
Add environment variables
SC_SERVER_HOSTandSC_SERVER_PORTfor sc-server configuration, with updates to documentation and source code.SC_SERVER_HOSTandSC_SERVER_PORTfor sc-server configuration indocker-compose.ymlandsc_server_module.cpp.changelog.mdandsc_machine.mdto include new environment variables.sc_server_module.cppto read host and port from environment variables.sc_server.cppto reflect "Host" instead of "Host name".This description was created by
for 768edc8. It will automatically update as commits are pushed.