6.9 -> 7.4 migration fixes #741
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tiran
requested changes
Apr 27, 2017
ipalib/util.py
Outdated
| @@ -520,10 +520,8 @@ def __get__(self, obj, cls): | |||
| return self.store[obj] | |||
|
|
|||
| def __set__(self, obj, value): | |||
| raise AttributeError("can't set attribute") | |||
| self.store[obj] = value | |||
There was a problem hiding this comment.
If you are going to turn the cached property into a writeable and deletable property, then you can remove the complicated weakref stuff and replace it with a simple non-data descriptor:
class cachedproperty(object):
slots = ('fget',)
def __init__(self, fget):
self.fget = fget
def __get__(self, obj, objtype=None):
if obj is None:
return self
value = self.fget(obj)
setattr(obj, self.fget.__name__, value)
return value
There was a problem hiding this comment.
ah, and add self.__doc__ = fget.__doc__.
There was a problem hiding this comment.
Decided to go with a simpler approach, pylint was a hater anyway.
|
For the record - the tests are passing on my machine, etwas stimmt hier nicht. |
tiran
reviewed
Apr 28, 2017
| """ | ||
| if self._ca_host is not None: | ||
| return self._ca_host |
stlaz
force-pushed
the
migration
branch
2 times, most recently
from
1ccd4c1
to
225fc31
Compare
|
It failed to me |
|
This was supposed to be fixed by the patch and worked for me, it seems that I may need to investigate it further. |
Refresh the ca_host property of the Dogtag's RestClient class when it's requested as a context manager. This solves the problem which would occur on DL0 when installing CA which needs to perform a set of steps against itself accessing 8443 port. This port should however only be available locally so trying to connect to remote master would fail. We need to make sure the right CA host is accessed. https://pagure.io/freeipa/issue/6878
The cachedproperty class was used in one special use-case where it only caused issues. Let's get rid of it. https://pagure.io/freeipa/issue/6878
|
Turns out I forgot to reorder the CA installation steps a bit. |
MartinBasti
added
ack
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allow rewriting of cached properties
Cached property should not be treated anyway special from a normal
property. If we need to rewrite/remove it, we should be able to do
just so.
Refresh Dogtag RestClient.ca_host property
Refresh the ca_host property of the Dogtag's RestClient class when
it's requested as a context manager.
This solves the problem which would occur on DL0 when installing
CA against an old master which does not have port 8443 accessible.
The setup tries to update the cert profiles via this port but
fail. This operation should be performed against the local instance
anyway.
https://pagure.io/freeipa/issue/6878