mgr/dashboard: dashboard help command showing wrong syntax for login-banner #46765
Conversation
Sarthak0702
requested review from
bryanmontalvan,
nizamial09,
sunilangadi2,
pereman2 and
epuertat
and removed request for
a team
| def set_login_banner_mgr(inbuf: str): | ||
| mgr.set_store(ITEM_KEY, inbuf) |
There was a problem hiding this comment.
These services look more anemic now. What about removing this file and moving this code inside the CLI commands (encapsulating service logic makes sense when it's complex, but in this case it's almost trivial: the service itself is provided by the mgr.set|get_store).
There was a problem hiding this comment.
I remember this was your concern in the original PR too. And I vouched for code duplication. But now I too feel this doesn't need a separate file.
|
|
||
|
|
||
| def get_login_banner_mgr(): | ||
| banner_text = mgr.get_store('custom_login_banner') | ||
| banner_text = mgr.get_store(ITEM_KEY) | ||
| logger.info('Reading custom login banner: %s', banner_text) |
There was a problem hiding this comment.
BTW this should better be a debug trace (info level is usually kept for operator meaningful events: service restarts, etc.,).
There was a problem hiding this comment.
ahh thats correct will update that too.
| def set_login_banner_mgr(inbuf: str, mgr_id: Optional[str] = None): | ||
| item_key = 'custom_login_banner' | ||
| if mgr_id is not None: | ||
| mgr.set_store(_get_localized_key(mgr_id, item_key), inbuf) |
There was a problem hiding this comment.
And yes, the localized here didn't make any sense (show a different message per mgr instance?).
There was a problem hiding this comment.
Yup, I confirmed this with @pereman2. We agreed different message per mgr instance was not required probably.
…banner Signed-off-by: Sarthak0702 <sarthak.dev.0702@gmail.com>
Sarthak0702
force-pushed
the
login-banner-cli-bug
branch
from
8669c4d
to
826741f
Compare
Fixes: https://tracker.ceph.com/issues/56137
Signed-off-by: Sarthak0702 sarthak.dev.0702@gmail.com
Contribution Guidelines
To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard cephadmjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume toxjenkins test windows