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

Should we convert this multinode parser (metadata.json) to class type ? #221

Closed
JoySnow opened this issue Jul 11, 2017 · 5 comments
Closed
Assignees

Comments

@JoySnow
Copy link
Collaborator

JoySnow commented Jul 11, 2017

This is related to PR: https://github.com/RedHatInsights/insights-core/pull/161

AFAIK:
Fact-1: we like class type parser better than the function type ones.
Fact-2: We'd love to do our best to get the plugins(the @rule reducers) out of troubles.( Let it handle less things.)

Well, based on Fact-1, I am converting this multinode parser from func to class type(suggested by @PaulWay ).
What did I do:
I wrap all the "metadata.json" parsing funcs into one class MetaData,
and add (rhev, osp, docker, metadata) as MetaData's attributes.
To fit this in insights-plugin side, plugins can be changed like this:
an example(https://github.com/RedHatInsights/insights-plugins/pull/143)

I test above changes by upload a rhev-coordinator archive to my local insight-engine server, and it works.
(Not guarantee of fully test.)

When thinking about Fact-2, yes, this change becomes a burden to plugins.
(The existence of metadata.json file means a hit parser for all the @rules which requires it.)

So my question here is:
Should we convert this multinode parser to class type ? Or someone has better ideas to achieve this.

@JoySnow
Copy link
Collaborator Author

JoySnow commented Jul 11, 2017

Please do ask me questions if I didn't express myself clearly.

@JoySnow
Copy link
Collaborator Author

JoySnow commented Jul 11, 2017

FYI, https://github.com/RedHatInsights/insights-plugins/issues/144 has a little bit connections with this one.

@csams
Copy link
Contributor

csams commented Jul 13, 2017

Hey @JoySnow, we agree this needs to be addressed and are taking steps in the master branch. We'd still like each environment to have a dedicated component (OSP, RHEV, etc.), and for cluster rules to get the top level component while host level rules get the relevant Child component for a particular host. Have a look at https://github.com/RedHatInsights/insights-core/blob/master/insights/parsers/multinode.py. Note that the metadata decorator is just shorthand for parser(requires=["metadata.json"]) and that we prevent unwanted components from resolving by raising SkipComponent.

@JoySnow
Copy link
Collaborator Author

JoySnow commented Jul 14, 2017

@csams Thanks for your explanation. Great, I can close this issue without changing anything here.

@JoySnow
Copy link
Collaborator Author

JoySnow commented Jul 14, 2017

Close this issue here.

@JoySnow JoySnow closed this as completed Jul 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants