Refresh url on access #107
Merged
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
sed-i
commented
Nov 25, 2022
sed-i
force-pushed
the
feature/refresh_url_on_access
branch
from
7290099 to
30bdecc
Compare
sed-i
requested review from
dstathis,
lucabello,
simskij,
PietroPasotti and
Abuelodelanada
| return None | ||
| return self.urls.get(self.charm.unit.name) | ||
| return urls.get(self.charm.unit.name) |
There was a problem hiding this comment.
do you just like this more, or is there a logic change I'm missing?
There was a problem hiding this comment.
If we could use walrus then I'd use walrus.
It's to avoid fetching the urls property twice.
It mattered more before I refactored.
| @@ -832,16 +831,16 @@ def urls(self) -> Dict[str, str]: | |||
|
|
|||
| May return an empty dict if the URLs aren't available yet. | |||
| """ | |||
| data = _type_convert_stored(self._stored.current_urls or {}) # type: ignore | |||
There was a problem hiding this comment.
can we get rid of stored altogether?
There was a problem hiding this comment.
Currently it's used to compare current vs. previous for emitting the custom events.
Do you mean use peer data instead?
PietroPasotti
approved these changes
Dec 1, 2022
This was referenced Dec 1, 2022
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.
Issue
canonical/catalogue-k8s-operator#3
Solution
Refresh url on access (example) by pre-fetching urls from relation data instead of stored state.
Context
See thread: canonical/catalogue-k8s-operator#4 (comment)
Testing Instructions
Deploy the bundle and see which URL catalogue has for prom: fqdn or ingress.
Release Notes
Prefetch urls from reldata instead of stored state.