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

removed Manager class metaclass and modified operation lookup to __ge… #200

Merged
merged 2 commits into from
Aug 23, 2018

Conversation

simingy
Copy link
Contributor

@simingy simingy commented Aug 4, 2017

OpExecutor metaclass's only functionality was to "patch" in vendor/standard operations.

using a metaclass to patch it in totally ignores the fact that Manager() class can be subclassed and subsequent operations (eg, get) modified through subclass methods.

this fixes it to use getattr to do a lookup if-and-only-if the operation isn't defined.

@simingy
Copy link
Contributor Author

simingy commented Aug 4, 2017

these are existing failures from the UT.

@einarnn einarnn added the 0.6.x label Jul 1, 2018
@einarnn
Copy link
Contributor

einarnn commented Aug 20, 2018

@siming85, this PR is now out of date with respect to the master branch. Can you please bring it up to date with the master branch?

Cheers,

Einar

@einarnn einarnn added the Waiting Waiting for a response from issue or PR author or for someone to volunteer to pick up an enhancement label Aug 20, 2018
@simingy
Copy link
Contributor Author

simingy commented Aug 22, 2018

merged

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 65.873% when pulling f5acec8 on siming85:master into 006bd36 on ncclient:master.

@einarnn einarnn merged commit a475d87 into ncclient:master Aug 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Waiting Waiting for a response from issue or PR author or for someone to volunteer to pick up an enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants