Match org.freedesktop.zbus.Error in error reporting #1251

mulkieran · 2026-01-06T22:56:54Z

Summary by CodeRabbit

Release Notes

Bug Fixes
- Fixed volume creation failure that occurred when creating multiple volumes
- Improved error message handling for property setting operations
Chores
- Updated build automation configuration

_{✏️ Tip: You can customize this high-level summary in your review settings.}

mulkieran · 2026-01-07T17:25:01Z

@coderabbitai review

coderabbitai · 2026-01-07T17:25:07Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

packit-as-a-service · 2026-01-07T17:26:28Z

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

sudo dnf install -y 'dnf*-command(copr)'
dnf copr enable packit/stratis-storage-stratis-cli-1251-copr_pull
And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

coderabbitai · 2026-01-07T17:29:11Z

Walkthrough

Configuration and error handling adjustments: removed manual trigger setting from copr_build job, eliminated DPClientSetPropertyContext-specific error message handling, and updated test expectations for volume creation from failure to success.

Changes

Cohort / File(s)	Change Summary
Configuration `.packit.yaml`	Removed `manual_trigger: true` from copr_build job; job now responds solely to PR-based triggering
Error Handling `src/stratis_cli/_error_reporting.py`	Removed dedicated error message construction block for DPClientSetPropertyContext property-setting failures in `_interpret_errors_2`; removed inline pragma comment from closing parenthesis
Test Update `tests/integration/logical/test_create.py`	Reversed test expectation in `test_creation` from expecting failure to success; replaced error assertion with command execution via TEST_RUNNER

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Use identifier for the name of the D-Bus error #1242: Modifies _interpret_errors_2 in error reporting module with refactoring of get_dbus_name() usage in the same function
Add additional pattern matches for error recognition #1240: Updates error handling branches in _interpret_errors_2 related to DP client property error contexts

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly references the main change: updating error reporting to handle org.freedesktop.zbus.Error. This aligns with the primary modification in src/stratis_cli/_error_reporting.py where zbus error matching was adjusted.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In @src/stratis_cli/_error_reporting.py:
- Around line 288-290: Tests don't exercise the branch where dbus_name ==
"org.freedesktop.zbus.Error" with a DPClientSetPropertyContext, so either re-add
the "# pragma: no cover" (with a short comment explaining why this zbus.Error
path is untested/untestable) to the conditional handling or add unit tests that
simulate DPClientSetPropertyContext raising/containing
"org.freedesktop.zbus.Error" and assert the handling code is executed;
specifically update the branch that checks dbus_name and
DPClientSetPropertyContext (symbols: dbus_name, DPClientSetPropertyContext,
"org.freedesktop.zbus.Error") to include the pragma+explanatory comment or
create a test that constructs a DPClientSetPropertyContext and triggers the zbus
error so coverage includes that branch.

🧹 Nitpick comments (1)

tests/integration/logical/test_create.py (1)
49-49: Consider using unpacking syntax for list construction.

The list concatenation can be simplified using the unpacking syntax for better readability.

Based on static analysis hints.
♻️ Proposed refactor
-        command_line = self._MENU + [self._POOLNAME] + self._VOLNAMES
+        command_line = [*self._MENU, self._POOLNAME, *self._VOLNAMES]

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d17f3c0 and f55b443.

📒 Files selected for processing (3)

.packit.yaml
src/stratis_cli/_error_reporting.py
tests/integration/logical/test_create.py

💤 Files with no reviewable changes (1)

.packit.yaml

🧰 Additional context used

🪛 Ruff (0.14.10)

tests/integration/logical/test_create.py

49-49: Consider [*self._MENU, self._POOLNAME, *self._VOLNAMES] instead of concatenation

Replace with [*self._MENU, self._POOLNAME, *self._VOLNAMES]

(RUF005)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)

GitHub Check: osh-diff-scan:fedora-rawhide-x86_64:copr_pull
GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
GitHub Check: test-runs (3.12)
GitHub Check: test-runs (3.13)

src/stratis_cli/_error_reporting.py

coderabbitai · 2026-01-07T17:29:15Z

tests/integration/logical/test_create.py

+        Creation of two volumes at once should succeed.
        """
        command_line = self._MENU + [self._POOLNAME] + self._VOLNAMES
-        self.check_error(StratisCliEngineError, command_line, _ERROR)
+        TEST_RUNNER(command_line)


⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's read the test file to see lines 47-50 and 109-110 cd tests/integration/logical && wc -l test_create.py

Repository: stratis-storage/stratis-cli

Length of output: 89

🏁 Script executed:

# Read the relevant sections of the test file sed -n '40,115p' tests/integration/logical/test_create.py

Repository: stratis-storage/stratis-cli

Length of output: 2641

🏁 Script executed:

# Search for comments about filesystem/volume creation restrictions in the entire repository rg -n -C3 -i 'only one filesystem|one volume per request|single filesystem per'

Repository: stratis-storage/stratis-cli

Length of output: 1257

Verify the actual restriction status and update the contradictory code accordingly.

The contradiction identified is real: Create3TestCase.test_creation() creates two volumes in a single call, but Create5TestCase.setUp() explicitly splits volume creation into separate calls with the comment "only one filesystem per request is currently allowed." This inconsistency indicates either:

The restriction has been lifted and the comment at lines 109-110 (and similar comment in test_destroy.py line 85) should be removed.

The test at lines 47-50 is incorrect and should split the volume creation to match the documented restriction.

Clarify which scenario applies and update the code and comments accordingly.

🧰 Tools

🪛 Ruff (0.14.10)

49-49: Consider [*self._MENU, self._POOLNAME, *self._VOLNAMES] instead of concatenation

Replace with [*self._MENU, self._POOLNAME, *self._VOLNAMES]

(RUF005)

mulkieran · 2026-01-07T17:34:11Z

Actionable comments posted: 2
🤖 Fix all issues with AI agents

🧹 Nitpick comments (1)
tests/integration/logical/test_create.py (1)> 49-49: Consider using unpacking syntax for list construction.
The list concatenation can be simplified using the unpacking syntax for better readability.
Based on static analysis hints.

♻️ Proposed refactor
-        command_line = self._MENU + [self._POOLNAME] + self._VOLNAMES
+        command_line = [*self._MENU, self._POOLNAME, *self._VOLNAMES]
📜 Review details

@coderabbitai I don't like the unpacking syntax, it doesn't look type-correct.

coderabbitai · 2026-01-07T17:35:03Z

Tip

For best results, initiate chat on the files or code changes.

@mulkieran, understood! I'll respect your preference for the list concatenation syntax. Both approaches are functionally equivalent and type-correct in Python, but if you find the concatenation style clearer for your codebase, that's perfectly reasonable.

✏️ Learnings added

Learnt from: mulkieran
URL: https://github.com/stratis-storage/stratis-cli/pull/1251

Timestamp: 2026-01-07T17:35:02.787Z
Learning: In the stratis-cli Python project, prefer list concatenation syntax (e.g., `self._MENU + [self._POOLNAME] + self._VOLNAMES`) over unpacking syntax (e.g., `[*self._MENU, self._POOLNAME, *self._VOLNAMES]`) for list construction.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

At present, the zbus D-Bus error is the only error that can be returned when setting a property. Signed-off-by: mulhern <[email protected]>

Signed-off-by: mulhern <[email protected]>

mulkieran added this to the v3.9.0 milestone Jan 6, 2026

mulkieran self-assigned this Jan 6, 2026

mulkieran added this to 2026January Jan 6, 2026

mulkieran moved this to In Progress in 2026January Jan 6, 2026

mulkieran force-pushed the additional-zbus-interfaces branch 2 times, most recently from 23aa537 to 0045d67 Compare January 7, 2026 01:18

coderabbitai bot reviewed Jan 7, 2026

View reviewed changes

mulkieran force-pushed the additional-zbus-interfaces branch from f55b443 to 3527c55 Compare January 8, 2026 16:54

mulkieran added 3 commits January 8, 2026 14:59

Only match org.freedesktop.zbus.Error for property setting

d9a0b5e

At present, the zbus D-Bus error is the only error that can be returned when setting a property. Signed-off-by: mulhern <[email protected]>

tests: Change creation of multiple file systems to expect success

351c1a4

Signed-off-by: mulhern <[email protected]>

TEMPORARY: remove manual trigger

031d1cf

Signed-off-by: mulhern <[email protected]>

mulkieran force-pushed the additional-zbus-interfaces branch from 3527c55 to 031d1cf Compare January 8, 2026 19:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Match org.freedesktop.zbus.Error in error reporting #1251

Match org.freedesktop.zbus.Error in error reporting #1251

Uh oh!

mulkieran commented Jan 6, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

mulkieran commented Jan 7, 2026

Uh oh!

coderabbitai bot commented Jan 7, 2026

Uh oh!

packit-as-a-service bot commented Jan 7, 2026

Uh oh!

coderabbitai bot commented Jan 7, 2026

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot Jan 7, 2026

Uh oh!

mulkieran commented Jan 7, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Match org.freedesktop.zbus.Error in error reporting #1251

Are you sure you want to change the base?

Match org.freedesktop.zbus.Error in error reporting #1251

Uh oh!

Conversation

mulkieran commented Jan 6, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

mulkieran commented Jan 7, 2026

Uh oh!

coderabbitai bot commented Jan 7, 2026

Uh oh!

packit-as-a-service bot commented Jan 7, 2026

Uh oh!

coderabbitai bot commented Jan 7, 2026

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

mulkieran commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mulkieran commented Jan 6, 2026 •

edited by coderabbitai bot

Loading

mulkieran commented Jan 7, 2026 •

edited

Loading