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

Add decorator for service & node name to allow using node metadata #70

Merged
merged 1 commit into from
Oct 5, 2020

Conversation

PHBourquin
Copy link
Contributor

In case meta needs to be used in place of node.name

Copy link
Contributor

@pierresouchay pierresouchay left a comment

Choose a reason for hiding this comment

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

Good, but I definitely prefer creating an element (we would add icons and similar stuff easily then)

@@ -36,6 +36,13 @@ function usefullLinksGenerator(instance, serviceName, node_meta_info) {
return top;
}

/**
* instanceNameDecorator is called to resolve instance name.
Copy link
Contributor

Choose a reason for hiding this comment

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

it resolves and display the nodeName, instance is too vague

/**
* instanceNameDecorator is called to resolve instance name.
*/
function instanceNameDecorator(instance, nodemeta) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather call it: createNodeDisplayElement(nodeName, nodeMeta)

and return by default : document.createTextNode(nodeName) => So it can be used to display specific icons for instance

instanceLink.setAttribute('target', '_blank');
}
instanceLink.appendChild(document.createTextNode(nodename));
var name = instanceNameDecorator(node['Name'], node['Meta'])
instanceLink.appendChild(document.createTextNode(name));
Copy link
Contributor

Choose a reason for hiding this comment

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

just do:

instanceLink.appendChild(createNodeDisplayElement(node['Name'], node['Meta']))

instanceLink.appendChild(document.createTextNode(instance.name + appendPort));
var nodemeta = (node_info != null) ? node_info.meta : null;
var name = instanceNameDecorator(instance.name, nodemeta)
instanceLink.appendChild(document.createTextNode(name + appendPort));
Copy link
Contributor

Choose a reason for hiding this comment

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

Just do:

instanceLink.appendChild(createNodeDisplayElement(instance.name, nodemeta))

In case meta needs to be used in place of node.name
@pierresouchay pierresouchay merged commit 3ff7cae into criteo:master Oct 5, 2020
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

Successfully merging this pull request may close these issues.

2 participants