Allow plugins to bring their own miq_reports #19391
Conversation
agrare
force-pushed
the
pluggable_miq_reports
branch
2 times, most recently
from
5b550a3
to
54b4c44
Compare
agrare
force-pushed
the
pluggable_miq_reports
branch
3 times, most recently
from
0b23020
to
ac0d3f0
Compare
agrare
force-pushed
the
pluggable_miq_reports
branch
2 times, most recently
from
e0f4511
to
b2baa6c
Compare
app/models/miq_report/seeding.rb
Outdated
| Dir.glob(REPORT_DIR.join("**/*.yaml")).sort + Dir.glob(COMPARE_DIR.join("**/*.yaml")).sort | ||
| end | ||
|
|
||
| def seed_plugin_files | ||
| Vmdb::Plugins.flat_map do |plugin| | ||
| Dir.glob(plugin.root.join("#{RELATIVE_REPORT_DIR}/**/*.yaml")).sort + Dir.glob(plugin.root.join("#{RELATIVE_COMPARE_DIR}/**/*.yaml")).sort |
There was a problem hiding this comment.
seed_core_files and seed_plugin_files seem pretty similar. Wonder if you could do something like this instead:
def seed_files
Vmdb::Plugins.flat_map.unshift(Rails) do |plugin|
seed_files_for plugin.root
end
end
def seed_files_for pathname
Dir.glob(dir.join("product", "{report|compare}", "**", "*.yaml")).sort
end
There was a problem hiding this comment.
Only weird part might be the sorting, but I'm not sure that's important with respect to reports coming before compare or not.
app/models/miq_report/seeding.rb
Outdated
| Dir.glob(REPORT_DIR.join("**/*.yaml")).sort + Dir.glob(COMPARE_DIR.join("**/*.yaml")).sort | ||
| end | ||
|
|
||
| def seed_plugin_files | ||
| Vmdb::Plugins.flat_map do |plugin| | ||
| Dir.glob(plugin.root.join("#{RELATIVE_REPORT_DIR}/**/*.yaml")).sort + Dir.glob(plugin.root.join("#{RELATIVE_COMPARE_DIR}/**/*.yaml")).sort |
There was a problem hiding this comment.
I want to avoid the "product" part in the plugin path...I've been trying to standardize on /content/thing, so in this case I'd expect for plugins /content/reports and content/compare.
There was a problem hiding this comment.
Hm okay, I assume we'd want to keep the current reports in product/ and only the plugin reports would be in content/?
There was a problem hiding this comment.
Yeah for now they will stay in product in core. Over time I'd like to move them to either a content directory in core, or a content directory in manageiq-content.
There was a problem hiding this comment.
Hm okay, I assume we'd want to keep the current reports in product/ and only the plugin reports would be in content/?
@agrare My 2cents would be to move them in both. Makes the code here easier to read, and just keeps things consistent between plugin and core.
There was a problem hiding this comment.
I'd rather not move the reports from core right now, particularly because reports are central to how the UI works and that's where they know them to exist.
agrare
force-pushed
the
pluggable_miq_reports
branch
from
b2baa6c
to
6aabf19
Compare
| elsif File.dirname(path).include?("compare") | ||
| path.split("compare/").last | ||
| else | ||
| path |
There was a problem hiding this comment.
I realized that seed_filename is sometimes passed not the full path but just the filename from the db record here and here, so it is possible to not have either "reports" or "compare" in the path.
Since those records have the filename set by seed_filename(path) I don't think we could index_by(&:filename) which simplifies this method but want to check to see if I'm missing something.
Check vmdb::plugins for product/reports and product/compare when seeding MiqReports
agrare
force-pushed
the
pluggable_miq_reports
branch
from
6aabf19
to
4b8fd72
Compare
|
Checked commit agrare@4b8fd72 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 |
|
@agrare Can you run the UI specs as well since they have a special reports spec there? |
Check vmdb::plugins for product/reports and product/compare when seeding
MiqReports
#14840