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.

Sign up for GitHub

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

Jump to bottom

Fixed issues related to etcd_server parameter #109

Closed
wants to merge 1 commit into from
Closed

Fixed issues related to etcd_server parameter #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions tendrl/commons/flows/base_flow.py
View file
Open in desktop
Expand Up @@ -156,18 +156,19 @@ def extract_atom_details(self, atom_name):
namespace = atom_name.split('.objects.')[0]
object_name = atom_name.split('.objects.')[1].split('.atoms.')[0]
atoms = self.definitions[namespace]['objects'][object_name]['atoms']
atom = atoms[atom_name.split('.')[-1]]
atom = atoms[atom_name.split('.')[-2]]
Copy link
Contributor

Choose a reason for hiding this comment

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

In https://github.com/anmolbabu/performance_monitoring/blob/bc6e351e2f268aaf1c18d770a3fcf367ad33fe2c/tendrl/performance_monitoring/manager/tendrl_definitions_performance_monitoring.py I didn't require this change as I changed my definition file itself to suit this. So in my definition file I changed the atom names under the objects to class names instead of file names because as per the split in above line it turns out to be looking for the last part of the module path which is the class name and I verified the change to be working.please suggest what is the decided/better approach so that we can make changes accordingly

Copy link
Member Author

Choose a reason for hiding this comment

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

I am ok with anything. The point is we are mixing uppercase and lowercase a lot for object and atom names. I personally feel we should restrict to uppercase for only object names and keep atom names lowercase only and as in above fix we are actually getting the module name for the atom details to be loaded from definitions.
@r0h4n @brainfunked @nthomas-redhat comments?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please check Tendrl/node-agent#147 for final naming. The problem needs to be tackled at both the source code side and the Definitions side.

The above PR will clear the confusion, keep an eye

return atom.get('name'), atom.get('enabled'), atom.get('help'), \
atom.get('inputs'), atom.get('outputs'), atom.get('uuid')
atom.get('inputs'), atom.get('outputs'), atom.get('uuid'), \
atom.get('run')

# Executes a givem atom specific by given full module name "mod"
# It dynamically imports the atom class from module as the_atom
# and executes the function run() on the instance of same
def execute_atom(self, mod):
if "tendrl" in mod and "atoms" in mod:
atom_name, enabled, help, inputs, outputs, uuid = \
atom_name, enabled, help, inputs, outputs, uuid, atom_mod = \
self.extract_atom_details(mod)
the_atom = import_utils.load_abs_class(mod)
the_atom = import_utils.load_abs_class(atom_mod)
try:
ret_val = the_atom(
atom_name,
Expand Down
2 changes: 1 addition & 1 deletion tendrl/commons/manager/rpc_job_process.py
View file
Open in desktop
Expand Up @@ -104,7 +104,7 @@ def invoke_flow(self, flow_name, job, definitions):
the_flow = import_utils.load_abs_class(flow_name)
return the_flow(flow_name, atoms, help, enabled, inputs, pre_run,
post_run, type, uuid, job['parameters'],
job, self.config, definitions).run()
job, definitions).run()

def extract_flow_details(self, flow_name, definitions):
try:
Expand Down
15 changes: 8 additions & 7 deletions tendrl/commons/tests/test_base_flow.py
View file
Open in desktop
Expand Up @@ -58,9 +58,9 @@ def setup_method(self):
}

self.flow_pre_run = \
['tendrl.dummymodule.objects.myobject.atoms.pre_run1']
['tendrl.dummymodule.objects.myobject.atoms.pre_run1.PreRun1']
self.flow_post_run = \
['tendrl.dummymodule.objects.myobject.atoms.post_run1']
['tendrl.dummymodule.objects.myobject.atoms.post_run1.PostRun1']
self.flow_parameters = {
'Tendrl_context.cluster_id':
"61959242-628f-4847-a5e2-2c8d8daac0ab",
Expand Down Expand Up @@ -148,7 +148,8 @@ def setup_method(self):
'flows': {
'dummy_flow': {
'atoms': [
'tendrl.dummymodule.objects.myobject.atoms.atom1'
'tendrl.dummymodule.objects.myobject.atoms.atom1.'
'Atom1'
],
'help': 'dummy_flow',
'enabled': True,
Expand All @@ -161,11 +162,11 @@ def setup_method(self):
},
'pre_run': [
'tendrl.dummymodule.objects.myobject.atoms.'
'pre_run1'
'pre_run1.PreRun1'
],
'post_run': [
'tendrl.dummymodule.objects.myobject.atoms.'
'post_run1'
'post_run1.PostRun1'
],
'run': 'tendrl.dummymodule.flows.dummy_flow.DummyFlow',
'type': 'Create',
Expand Down Expand Up @@ -202,9 +203,9 @@ def test_run(self):
assert self.flow_obj.execute_atom.return_value is True

def test_extract_atom_details(self):
name, enabled, help, inputs, outputs, uuid = \
name, enabled, help, inputs, outputs, uuid, mod = \
self.flow_obj.extract_atom_details(
"tendrl.dummymodule.objects.myobject.atoms.pre_run1",
"tendrl.dummymodule.objects.myobject.atoms.pre_run1.PreRun1",
)
assert name == "pre_run1"
assert enabled is True
Expand Down
4 changes: 2 additions & 2 deletions tendrl/commons/utils/import_utils.py
View file
Open in desktop
Expand Up @@ -7,6 +7,6 @@ def load_abs_module(module_abs_path):


def load_abs_class(class_abs_path):
kls_name = class_abs_path.split(".")[:-1]
kls_name = class_abs_path.split(".")[-1]
module_name = ".".join(class_abs_path.split(".")[:-1])
return getattr(kls_name, load_abs_module(module_name))
return getattr(load_abs_module(module_name), kls_name)