Skip to content

Add method to convert set options dictionary to string format for Docker Buildx Bake #361

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.

Sign up for GitHub

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

Open
ianpittwood wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add method to convert set options dictionary to string format for Docker Buildx Bake #361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e91f5cd
Add method to convert set options dictionary to string format for Doc…
ianpittwood Feb 19, 2026
604d282
Merge branch 'main' into fix-bake-set-temp-registry
ianpittwood Feb 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions posit-bakery/posit_bakery/config/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,9 +897,7 @@ def build_targets(
)
set_opts = None
if self.settings.temp_registry is not None and push:
set_opts = {
"*.output": [{"type": "image", "push-by-digest": True, "name-canonical": True, "push": True}]
}
set_opts = {"*.output": {"type": "image", "push-by-digest": True, "name-canonical": True, "push": True}}
bake_plan.build(
load=load,
push=push,
Expand Down
19 changes: 19 additions & 0 deletions posit-bakery/posit_bakery/image/bake/bake.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,24 @@ def remove(self):
"""Delete the bake plan file if it exists."""
self.bake_file.unlink(missing_ok=True)

@staticmethod
def _set_opts_dict_to_str(set_opts: dict[str, Any]) -> dict[str, str]:
"""Convert a dictionary of set options to a comma-delimited, key=value string format for Docker Buildx Bake.

:param set_opts: A dictionary of set options to convert.

:return: A dictionary of set options with string values.
"""
for opt, data in set_opts.items():
if isinstance(data, list):
set_opts[opt] = ",".join(data)
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The join operation assumes all list items are strings. If the list contains non-string types (e.g., numbers, booleans), this will raise a TypeError. Consider converting list items to strings: ",".join(str(item) for item in data). Note that boolean values should be lowercased for Docker compatibility, so you may need ",".join(str(item).lower() if isinstance(item, bool) else str(item) for item in data).

Suggested change
set_opts[opt] = ",".join(data)
set_opts[opt] = ",".join(
str(item).lower() if isinstance(item, bool) else str(item)
for item in data
)

Copilot uses AI. Check for mistakes.
elif isinstance(data, dict):
set_opts[opt] = ",".join(f"{k}={v}" for k, v in data.items())
else:
set_opts[opt] = str(data)
Comment on lines +215 to +219
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When converting values to strings, Python boolean values will be rendered as "True" or "False" (capitalized). However, Docker Buildx expects lowercase boolean values ("true", "false") in set options. This affects both the dictionary values in line 203 and the else branch in line 205. Consider adding special handling for boolean values: str(v).lower() if isinstance(v, bool) else str(v) to ensure proper formatting for Docker.

Suggested change
set_opts[opt] = ",".join(data)
elif isinstance(data, dict):
set_opts[opt] = ",".join(f"{k}={v}" for k, v in data.items())
else:
set_opts[opt] = str(data)
set_opts[opt] = ",".join(
str(v).lower() if isinstance(v, bool) else str(v) for v in data
)
elif isinstance(data, dict):
set_opts[opt] = ",".join(
f"{k}={str(v).lower() if isinstance(v, bool) else str(v)}"
for k, v in data.items()
)
else:
set_opts[opt] = str(data).lower() if isinstance(data, bool) else str(data)

Copilot uses AI. Check for mistakes.

return set_opts
Comment on lines +213 to +221
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method mutates the input dictionary directly by modifying set_opts in place. This can lead to unexpected side effects if the caller retains a reference to the original dictionary. Consider creating a new dictionary instead of modifying the input. For example, you could initialize a new dictionary result = {} and populate it in the loop, then return result instead of set_opts.

Suggested change
for opt, data in set_opts.items():
if isinstance(data, list):
set_opts[opt] = ",".join(data)
elif isinstance(data, dict):
set_opts[opt] = ",".join(f"{k}={v}" for k, v in data.items())
else:
set_opts[opt] = str(data)
return set_opts
result: dict[str, str] = {}
for opt, data in set_opts.items():
if isinstance(data, list):
result[opt] = ",".join(data)
elif isinstance(data, dict):
result[opt] = ",".join(f"{k}={v}" for k, v in data.items())
else:
result[opt] = str(data)
return result

Copilot uses AI. Check for mistakes.
Comment on lines +205 to +221
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new _set_opts_dict_to_str method lacks test coverage. Given that the repository has comprehensive test coverage for similar methods (as seen in test_bake.py with tests for other BakePlan methods), consider adding tests to verify the correct conversion of different data types (lists, dicts, strings, numbers, booleans) to their string representations. This is especially important since this method handles the critical task of formatting parameters for Docker Buildx Bake.

Copilot uses AI. Check for mistakes.

def build(
self,
load: bool = True,
Expand All @@ -228,6 +246,7 @@ def build(
_set["*.cache-to"] = cache_to
if set_opts:
_set.update(set_opts)
_set = self._set_opts_dict_to_str(_set)

python_on_whales.docker.buildx.bake(files=[self.bake_file.name], load=load, push=push, cache=cache, set=_set)
if clean_bakefile:
Expand Down