BO-82 "cm::services::stream" extend with status-secret token param #1664
Conversation
@tomaszdurka please review |
class cm::services::stream( | ||
$port = 8090, | ||
class cm::services::stream ( | ||
$port = 8090, | ||
$ssl_cert, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: should you order params by non-optonial in front?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am OK to have it optional. I thought you wanna enforce this token for higher security.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, tbh. I am just describing possibilities from socket-redis library itself.
I guess we could have done that there.
On the other hand I think it's nice to be able to start this library locally without any extra configuration.
$statusPort = '8085', | ||
$logDir = '/var/log/socket-redis', | ||
$statusPort = '8085', | ||
$statusToken = 'supersecret', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$socket_ports = [8091, 8092, 8093, 8094], | ||
$status_port = 8085 | ||
$status_port = 8085, | ||
$status_token = 'supersecret', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make it optional here too, but as you wish.
@tomaszdurka please re-review, all is optional. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@tomaszdurka please re-review again, there was missing secret config for daemon! |
ok, but still failing |
retest this please |
@tomaszdurka specs are fine, CI was having troubles again. |
lgtm then |
Depends on cargomedia/bipbip#175