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

ipa help doesn't always work #701

Closed
wants to merge 2 commits into from
Closed

ipa help doesn't always work #701

wants to merge 2 commits into from

Conversation

neffs
Copy link
Contributor

@neffs neffs commented Apr 7, 2017

ipa help will not work when calling it when no schema is cached.

Signed-off-by: David Kreitschmann <david@kreitschmann.de>
@stlaz stlaz requested a review from a user April 12, 2017 11:15
@martbab
Copy link
Contributor

martbab commented May 26, 2017

@neffs please fix pylint error reported in Travis CI:

************* Module ipaclient.remote_plugins.schema

ipaclient/remote_plugins/schema.py:519: [E1101(no-member), Schema.get_help] Instance of 'dict' has no 'decode' member)

make: *** [pylint] Error 2

Makefile:1175: recipe for target 'pylint' failed

@neffs
Copy link
Contributor Author

neffs commented May 26, 2017

@martbab
Not sure why pylint is reporting this, it looks fine to me and I didn't change the reported lines.

        if isinstance(self._help, bytes):
            self._help = json.loads(self._help.decode('utf-8'))

maybe we can trick it with a change like this (similar to read_namespace_member):

        value = self._help
        if isinstance(value, bytes):
            self._help = json.loads(value.decode('utf-8'))

@neffs
Copy link
Contributor Author

neffs commented Jun 9, 2017

@martbab I pushed this additional change but pylint still reports this error which is clearly wrong because the line is only executed if it is an instance of bytes. Could you please provide some directions?
I only have the code on my mac and unfortunately can't run the tests locally.

@ghost
Copy link

ghost commented Jun 12, 2017

@neffs Hi, thanks for the patch. As for the pylint issue it happens sometimes that pylint gets confused even in situations that seems perfectly clear. This will be probably the case because it complains about member of dict instance in branch when it's guaranteed to be bytes instance.

I propose following "fix". Just tell pylint to don't worry about this case.

 if isinstance(self._help, bytes):
            self._help = json.loads(
                self._help.decode('utf-8')  # pylint: disable=no-member
            )

@ghost
Copy link

ghost commented Jun 12, 2017

@neffs Thanks for the fix. Could you please drop the second commit ("Fix pylint error in get_help function"), rebase and force-push? The commit gets completely reverted in the third one so it has no value.

@ghost ghost closed this Jun 13, 2017
@ghost ghost reopened this Jun 13, 2017
@ghost
Copy link

ghost commented Jun 13, 2017

Works for me, thanks.

(Sorry for the close/reopen, it's too early in the morning here :-)

@ghost ghost added the ack Pull Request approved, can be merged label Jun 13, 2017
@martbab martbab added the pushed Pull Request has already been pushed label Jun 15, 2017
@martbab
Copy link
Contributor

martbab commented Jun 15, 2017

master:

  • d5bb541 Store help in Schema before writing to disk
  • bf0ba9b Disable pylint in get_help function because of type confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
2 participants