Restore the original functionality of env and plugins commands
#278
Conversation
This reverts commit 1166fbc. The proper fix is to restore pre-thin client behavior of commands inheriting from LocalOrRemote class. https://fedorahosted.org/freeipa/ticket/6490
martbab
force-pushed
the
api-env-fixes
branch
from
4eeb3fb
to
69a2bf0
Compare
During thin client refactoring, LocalOrRemote class implementation of `run` method was overriden by default Command implementation during instantiation of client plugins from schema. This caused these commands to always forward this request to IPA master. This patch restores the original behavior: unless `--server` option was specified, the commands will always print out local config. https://fedorahosted.org/freeipa/ticket/6490
martbab
force-pushed
the
api-env-fixes
branch
from
69a2bf0
to
5b38246
Compare
Make the code moved from `ipaserver/plugins` pep-8 conformant. https://fedorahosted.org/freeipa/ticket/6490
martbab
force-pushed
the
api-env-fixes
branch
from
5b38246
to
75ba652
Compare
|
|
||
| return dict( | ||
| result=result, | ||
| ) |
There was a problem hiding this comment.
I think this code should rather be in ipalib.plugins
There was a problem hiding this comment.
Why should it be in ipalib.plugins? can you explain your reasons better?
There was a problem hiding this comment.
It was originally a plugin and still keeps plugin-like behavior so I thought it should rather be there but there's no other reason.
There was a problem hiding this comment.
Well since these particular classes must behave as both server and client plugins at the same time, you would then have this code copy-pasted in two places. I rather opted to keep implementation in one place and a) registering it on server, and b) overriding it on client.
| @@ -26,26 +26,31 @@ class env(LocalOrRemote): | |||
| ) | |||
|
|
|||
| takes_options = LocalOrRemote.takes_options + ( | |||
| Flag('all', | |||
| Flag( | |||
There was a problem hiding this comment.
Wouldn't it be better to go like this?
Flag('all',
cli_name='all',
...
)Otherwise it looks a bit ridiculous as you're saving about 1-2 spaces here :)
There was a problem hiding this comment.
If you look at some other newly written plugins (e.g. ipaserver/plugins/server.py) you will se that I have used the option formatting constistent with them and I would rather keep it like that.
|
|
This reverts commit 1166fbc "Add 'ipa
localenv' subcommand" and instead fixes the command to be executed locally
unless
--serveroption is given.Note than
pluginscommand fails due tohttps://fedorahosted.org/freeipa/ticket/6513 but now at least it fails either
locally or on server-side :).
https://fedorahosted.org/freeipa/ticket/6490