fix: respect config-bucket precedence and deduplicate buckets in recompute - #188
Merged
Merged
Conversation
The get_statuses(recompute=True) path in the S3 provider diverged from reconcile_buckets in two ways: 1. It re-evaluated relation-requested buckets even when a config bucket was set, violating the documented precedence rule where the config bucket always overwrites requester-provided buckets. This could emit a spurious 'Could not ensure bucket(s)' status for a relation bucket that should have been ignored. 2. It did not de-duplicate bucket names, so multiple requirers asking for the same unavailable bucket produced messages like 'Could not ensure bucket(s): mlpipeline, mlpipeline'. This aligns the recompute path with reconcile_buckets: when a config bucket is set only that bucket is evaluated, and requested bucket lists are de-duplicated while preserving order. Fixes #187
mvlassis
changed the title
Batalex
approved these changes
Jul 28, 2026
Batalex
left a comment
Batalex
left a comment
Contributor
There was a problem hiding this comment.
LGTM. Very nice fix, thanks :)
I wonder if we need/want to release three new revisions (even if we cannot access them after 30 days) on every push on every PR. Just with the renovate setup running every month, that will amount to quite a huge number for something that is easily toggleable when we actually need it.
I'll approve the PR, let's see what a second review will say about this
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Overview
Fixes #187, where
s3-integratorends up blocked with a message like:Root cause
The
get_statuses()method diverges fromreconcile_buckets. According to the docs (sees3/README.md), consumer-specific bucket/path configuration should take precedence and overwrite any bucket requested by a requirer.get_statusesstill revaluates every relation-requested bucket, even if there is a config set and thus should have been ignored.No de-duplication when calculating the missing buckets, resulting in the same bucket name could appear more than once, producing the duplicated
'mlpipeline', 'mlpipeline'message.Notes
ci.yamlaction to release the 3 charms in a PR channel whenever a PR is raised.