errors: model ExceptionResourceLockConflict and ExceptionResourceNoAccess as typed exceptions#173
Conversation
…cess as typed exceptions
new_adt_error_from_xml() in sap/adt/errors.py walks ADTError.__subclasses__()
to map the SAP-side exception type id back to a Python class, so adding a
typed subclass is enough to make callers able to write a typed
'except ExceptionResourceLockConflict:' instead of regexing the message
text of a plain ADTError.
Two ADT exception types were missing from the existing four typed
wrappers (AlreadyExists, NotFound, CreationFailure, SaveFailure). Both
are reachable by ordinary CLI flows on modern S/4HANA (Cloud and gCTS-
managed) systems:
ExceptionResourceLockConflict
Raised when the ADT backend rejects a create/modify because the
lock state of the addressed resource - or one of its parents like
a software component or package - does not allow the change.
Common message text:
'No suitable software component is modifiable; cannot create object'
Hit on package or DDIC create calls under a parent that is not
modifiable for the current user.
ExceptionResourceNoAccess
Raised when the calling user has no access to the addressed
resource or transport request. On gCTS-managed systems the
common message is:
'Request <T> cannot be used since it is not assigned to repository <R>'
meaning the object's gCTS repository binding rejects the transport
that was picked (or the implicit one).
Both are net-additive: existing 'except ADTError:' blocks continue to
match (the new classes inherit from ADTError unchanged). Code that
wants type-based dispatch can now do it; code that doesn't can keep
ignoring the change.
Tests follow the existing test_parse_existing_package shape one-for-one
and use new fixtures in test/unit/fixtures_adt.py with sanitised placeholder
transport / repository names.
The fixture style (NAME='...') matches the pre-existing convention in
fixtures_adt.py rather than PEP-8 spaces around '=' - kept for diff
locality. Happy to switch on review request.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces two new ADT-specific exception classes ( ChangesADT Exception Classes and Tests
🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
What this PR does
new_adt_error_from_xml()insap/adt/errors.pywalksADTError.__subclasses__()to map the SAP-side exception type id back to a Python class. Adding a typed wrapper for an ADT exception is therefore a one-line declaration — the dispatcher finds it by name automatically.Four exception types were already typed (
ExceptionResourceAlreadyExists,ExceptionResourceNotFound,ExceptionResourceCreationFailure,ExceptionResourceSaveFailure). Two more are reachable on ordinary CLI flows on modern S/4HANA (Cloud and gCTS-managed) systems but were falling through to the genericADTError. This PR adds them.The two new exceptions
ExceptionResourceLockConflictRaised when the ADT backend rejects a create/modify because the lock state of the addressed resource — or one of its parents like a software component or package — does not allow the change.
Reproducer (live HTTP capture, just now):
Triggered by:
…on a S/4HANA system where the parent package is not modifiable for the calling user.
ExceptionResourceNoAccessRaised when the calling user has no access to the addressed resource or transport request. On gCTS-managed systems the typical message form is
Request <T> cannot be used since it is not assigned to repository <R>.Reproducer (live HTTP capture, just now):
Triggered by:
…on a gCTS-managed system where the class belongs to a repository the calling user is not in scope for.
The fixture in this PR uses sanitised placeholder names (
DEVK900042,sapcli_test_repo) so no SAP-internal identifiers leak to the public repo.Backward compatibility
Net additive.
except ADTError:blocks continue to match — the new classes inherit fromADTErrorunchanged. Code that already does typed dispatch on the four older exceptions is unaffected.Tests
New tests follow the existing
test_parse_existing_packageshape one-for-one intest/unit/test_sap_adt_errors.py.python -m unittest discover -b -s test/unit: master 2699 → branch 2701 (+2 new tests). The 4 failures + 16 errors that exist onmasterare pre-existing environmental issues intest_sap_cli_config/test_sap_config(Python 3.14 / OS error-message wording inFileNotFoundError); they reproduce identically on unmodifiedmaster.Lint
pylint --rcfile=.pylintrc sap/adt/errors.py→ 10.00/10.flake8 --config=.flake8 sap/adt/errors.py→ clean.The two new fixture entries in
test/unit/fixtures_adt.pyuse the existingNAME='...'no-space convention of that file (rather than PEP-8NAME = '...') for diff locality with the surrounding fixtures. Happy to switch on review request.Net diff
Why this matters in practice
LLM coding agents using sapcli (e.g. Claude Code with the sapcli plugin) currently have to regex on
ADTErrormessage text to differentiate "no suitable SC modifiable" from "wrong gCTS repo binding" from "object not found". With these subclasses, agents can branch by Python type and offer the right remediation (add--record-changes, escalate to repo admin, create-then-write, etc.) deterministically.This PR is the upstream prerequisite for a transport-pick skill update on the plugin side.