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
Fixed issues related to etcd_server parameter #109
Conversation
ca2cd01
to
f19c9f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
f19c9f7
to
990e11d
Compare
990e11d
to
c5e0156
Compare
d6848ee
to
5f80eeb
Compare
6d398ee
to
0f132bd
Compare
tendrl-bug-id: Tendrl#114 Signed-off-by: Shubhendu <shtripat@redhat.com>
0f132bd
to
82f1ccb
Compare
@@ -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]] |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
PR not relevant any more |
tendrl-bug-id: #114
Signed-off-by: Shubhendu shtripat@redhat.com