Resolve out of sync presence map #366
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
The following attempts to keep the presence map in-sync with the actual data after a normalisation. During a normalisation "scale" is swapped out for "num_units". This is fine, except when it comes to overlays you now don't know if scale or num_units was used for the overlays and subsequent scale calls are now not used. This is actually a problem with the library itself. There should be an internal representation of a bundle that carries all this important information, which should include schemas etc. At the very end of the reading/parsing process we should hand back a clean version of that charm/bundle. That way we can make the bundle grow organically without the requirement of so many hacks. The code is simple, detect and resolve a normalisation and just insert the right value into the presence map before applying an overlay.
achilleasa
approved these changes
May 10, 2021
When dealing with deletions of applications for overlays, the presence map will have removed applications. In that case we need to not panic when attempting to resolve the fields for an out of sync map. It is much safer to always return a map back and then determine if the field present is on the empty FieldPresenceMap. We will fail a bit later on, but it means we don't have to force consumers of the forField check for existence of a value.
|
|
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.
The following attempts to keep the presence map in sync with the actual
data after normalization. During a normalization "scale" is swapped
out for "num_units". This is fine, except when it comes to overlays you
now don't know if scale or num_units was used for the overlays and
subsequent scale calls are now not used.
This is actually a problem with the library itself. There should be an
internal representation of a bundle that carries all this important
information, which should include schemas etc. At the very end of the
reading/parsing process, we should hand back a clean version of that
charm/bundle. That way we can make the bundle grow organically without
the requirement of so many hacks.
The code is simple, detect and resolve a normalization and just insert
the right value into the presence map before applying an overlay.
Bug: https://bugs.launchpad.net/juju/+bug/1884465