Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mgr/dashboard: add configuration setting browser #20043

Merged
merged 1 commit into from Feb 9, 2018

Conversation

Rubab-Syed
Copy link
Contributor

@Rubab-Syed Rubab-Syed commented Jan 22, 2018

@Rubab-Syed
Copy link
Contributor Author

config

@@ -187,9 +187,13 @@ PyObject *ActivePyModules::get_python(const std::string &what)
}
});
return f.get();
} else if (what == "config") {
} else if (what.substr(0, 6) == "config") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what.startswith("config") would be clearer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I will have to add some C++ library for this. Should I use boost or something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silly me, I thought I was reading python. Never mind, this can stay as is!

content_data.options_list = content_data.options.options;
}
for (var opt of content_data.options.options) {
if (service == "any" && opt.level == level) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When someone selects a level, they should also get all the lower levels (i.e. if I select "advanced" then it should show both advanced and basic options)

<thead>
<tr>
<th>Name</th>
<th>desc</th>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Column headers should be full words where reasonable (i.e. Description rather than dec)

<div class="box" style="overflow:auto">
<div class="box-body">
<table class="table table-bordered">
<thead>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed, I think this table needs some constraints on the widths of columns so that really long names don't push the whole thing out too far.

f->open_array_section("options");
for (const auto& i: schema) {
const Option &opt = i.second;
f->open_object_section("option");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Option class has a dump method that you can use instead of dumping all fields explicitly here.

Signed-off-by: Rubab Syed<rubab.syed21@gmail.com>
@jcsp jcsp merged commit 4e2748d into ceph:master Feb 9, 2018
@epuertat
Copy link
Member

epuertat commented Aug 3, 2018

@Rubab-Syed @jcsp @smithfarm, dashboard_v2 relies on config_options method for building the configuration doc. page.

This commit does not alter any behaviour to the existing Ceph components, it simply adds the new config_options method to src/common/config.{cc,h}, and exposes it by Mgr Python interface src/mgr/ActivePyModules.cc. It also implements the dashboard_v1 config page.

Any objection to this backporting to Luminous for the ongoing dashboard_v2 backporting #23271?

@jcsp
Copy link
Contributor

jcsp commented Aug 6, 2018

No objection from me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants