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

Fix for implicit publish on add/remove node #94

Merged
merged 4 commits into from
Aug 17, 2016

Conversation

whisk3y
Copy link
Contributor

@whisk3y whisk3y commented Aug 17, 2016

@Marchowes @moogar0880 Can you guys check this, it's a quick change to the add_node/remove_node functions on TD. Currently it is hardcoded to publish immediately which is causing problems.

if self._implicitPublish is True:
publish = "Y"
else:
publish = "N"
Copy link
Contributor

Choose a reason for hiding this comment

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

You could simplify to publish = "Y" if self._implicitPublish else "N"

@Marchowes
Copy link
Contributor

Huh, yeah, this function is a bit different eh? No _update() implicit publish trickery since it calls the session directly.

Only publish(), delete(), and "Get" type calls should "ignore" implicitPublish. I did a quick skim through and found one other call (Line 4168) that needs this publish check. Not surprisingly it part of the Node Functionality.

Other than that, this change LGTM. +1

@whisk3y
Copy link
Contributor Author

whisk3y commented Aug 17, 2016

Merging PR with 2 reviews.

@whisk3y whisk3y merged commit 71dbcc1 into dyninc:master Aug 17, 2016
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.

None yet

3 participants