feat(generate): restructure file generation of conf and spec files

[hetangmodi-crest]

Issue number:

Summary

Changes

Please provide a summary of what's being changed

User experience

Please describe what the user experience looks like before and after this change

Checklist

If your change doesn't seem to apply, please leave them unchecked.

• I have performed a self-review of this change
• Changes have been tested
• Changes are documented
• PR title follows conventional commit semantics

[kkedziak-splunk]

I'm wondering if there is a need for that _set_attributes method, when we still define __init__ for the subclasses.

Or maybe we should at least get rid of __init__? Because this call to the superclass __init__ that needs to be added every time, looks weird.

[kkedziak-splunk]

Also I am not sure about using **kwargs. Maybe we should at least leave **kwargs in the base class but use real parameters in the overridden methods in subclasses?

[hetangmodi-crest]

Yes, we could merge _set_attributes and __init__, I kept them different as to define what needs to be initialized and what attributes need to be set for self for each individual classes. Should we merge them in __init__?

Currently, **kwargs is required by AppConf class to generate the details required for app.conf which comes from build.py (to avoid redoing of things), and perhaps in future for XML or Python or other file generation, we might require it. What are your thoughts on this?

[kkedziak-splunk]

We could for example at least leave def _set_attributes(self, **kwargs): but change the method signature in subclasses to for example: def _set_attributes(self): (when no parameters are needed), or def _set_attributes(self, some_param): - type checking should accept that, I think and we'll get better error messages when some parameter is missing.

[hetangmodi-crest]

Since the parent class is invoking the _set_attributes, it will pass **kwargs to whichever child class requires it. Moreover, I tried updating the method declaration to def _set_attributes(self) where **kwargs aren't required, and it raised a TypeError.

[kkedziak-splunk]

Do we actually need this class between the base class and the actual subclasses? It only introduces generate_conf and generate_conf_spec methods and calls them. But I see not every conf file needs the spec.

[hetangmodi-crest]

Yes, from code, this class is redundant. However, my thought process in the design were to create a hierarchy of classes with justifiable bifurcation so that it is easier to maintain the procedures of file that are being generated and the order they should be generated in. Should we remove these bifurcation layers then?

[hetangmodi-crest]

Closing the PR as this PR has been broken down into PRs - #1311, #1312, #1328 and #1329. Hence, closing this PR.