Initial ddl apireleases#151
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds API Release support: a new core module modeling ADT API release contracts with HTTP helpers, a CLI for listing/updating contract states, package re-exports, documentation updates, and comprehensive unit tests and fixtures. (No runtime-breaking public API changes beyond added exports.) Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI (apistate)
participant Conn as HTTP Connection
participant XML as XML (de)serializer
User->>CLI: apistate set --contract C0 --release-state RELEASED
CLI->>Conn: GET /adt/.../object?...
Conn-->>CLI: 200 API Release XML
CLI->>XML: deserialize -> ApiRelease
CLI->>CLI: copy contract, apply overrides
CLI->>Conn: POST /adt/.../c0/validationrun (body: payload XML)
Conn-->>CLI: 200 Validation XML
CLI->>XML: deserialize -> ApiReleaseValidation
alt validation contains errors
CLI-->>User: print errors, abort
else validation contains warnings (no errors)
CLI->>User: prompt (or --force)
alt user confirms / --force
CLI->>Conn: PUT /adt/.../c0?request=... (body: payload XML)
Conn-->>CLI: 200 Updated API Release XML
CLI->>XML: deserialize -> ApiRelease
CLI-->>User: print updated state
else
CLI-->>User: abort
end
else no validation messages
CLI->>Conn: PUT /adt/.../c0 (body: payload XML)
Conn-->>CLI: 200 Updated API Release XML
CLI->>XML: deserialize -> ApiRelease
CLI-->>User: print updated state
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
sap/adt/apirelease.py (1)
170-170: Unused constantVALID_CONTRACTS.
ContractKeyis the authoritative source; this tuple is duplicated state and is never referenced. Remove it to avoid drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sap/adt/apirelease.py` at line 170, Remove the duplicated, unused constant VALID_CONTRACTS from the module: delete the line defining VALID_CONTRACTS = ('c0', 'c1', 'c2', 'c3', 'c4') and any references to it; rely on the authoritative ContractKey definition instead (ensure no other code in this file references VALID_CONTRACTS and update tests or callers to use ContractKey if needed).test/unit/test_sap_cli_apirelease.py (1)
16-71: Move these XML fixtures into a guideline-compliant fixture module.This test mixes inline XML fixtures with imports from
fixtures_adt_apirelease; the visible imported fixture module also does not use the requiredtest_prefix. Please keep these payloads in a same-directory fixture module whose name follows the repository’s test fixture naming convention.As per coding guidelines, “Test fixtures should be located in the same directory as the test files (test/unit/)” and “Test fixtures should be stored as module files with the prefix test_”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/test_sap_cli_apirelease.py` around lines 16 - 71, The test currently declares inline XML constants (API_RELEASE_NO_CONTRACTS_XML, API_RELEASE_READONLY_XML, NO_BEHAVIOUR_XML) while also importing fixtures from fixtures_adt_apirelease (which lacks the required test_ prefix); move those inline XML strings into a new same-directory fixture module named with the test_ prefix (e.g., test_fixtures_adt_apirelease.py), place all related constants (API_RELEASE_NO_CONTRACTS_XML, API_RELEASE_READONLY_XML, NO_BEHAVIOUR_XML and the previously imported OBJECT_URI, API_RELEASE_RESPONSE_XML, etc.) into that module, then update the test file to import these symbols from the new test_fixtures module and remove the inline declarations; ensure the new module lives in test/unit/ and uses the exact symbol names used by the test so no other code changes are required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@doc/architecture.md`:
- Line 30: Fix the typo in the documentation sentence so "properterty" reads
"property" — update the line that mentions the Object ADT path and the
`full_adt_uri` property to: "The complete Object ADT path is available in the
property `full_adt_uri`."
In `@sap/adt/apirelease.py`:
- Around line 270-297: The URL segment for the contract is built incorrectly
because validate_api_release uses str(contract).lower(), which yields
"contractkey.c1" for a ContractKey enum; change validate_api_release to
normalize the contract into the raw token (e.g., use contract.value when
contract is a ContractKey, or accept a string if passed), and update the
signature to accept Union[ContractKey, str] (or change set_api_release to accept
ContractKey for consistency) so both functions build the path with the plain
contract token (like "c1"); also remove or use VALID_CONTRACTS sensibly if meant
for validation.
In `@sap/cli/apirelease.py`:
- Around line 217-219: The call to validate_api_release currently passes
contract_key.value (a string) but the function is declared to accept a
ContractKey; change the call to pass the ContractKey object (contract_key)
instead of contract_key.value so the argument matches the declared parameter
type of validate_api_release; update any nearby variable usage in the same call
site (the call in apirelease.py where validate_api_release(connection,
obj.full_adt_uri, contract_key.value, payload) is invoked) to use contract_key
and ensure types/imports still resolve after the upstream fix to the function
signature.
- Around line 152-158: The _build_payload function dereferences target.status
after calling copy_contract which can return a contract with status None; before
setting target.status.state_description in _build_payload, check if
target.status is None and if so assign a new ContractStatus() (or otherwise
initialize an empty status object), then set state_description to None;
alternatively ensure copy_contract always returns a contract whose status is a
ContractStatus, but for a minimal fix update _build_payload to guard
target.status and initialize it when missing (referencing _build_payload,
copy_contract, target.status, and ContractStatus).
- Around line 145-149: The code currently indexes YES_NO_MAP with user input
(YES_NO_MAP[args.cloud_dev.lower()] and YES_NO_MAP[args.key_user_apps.lower()])
which can raise an unhandled KeyError for invalid values; instead, validate and
convert the inputs and raise a sap.errors.SAPCliError on invalid values before
assigning to target.use_in_sap_cloud_platform and target.use_in_key_user_apps.
Implement a small helper (e.g., normalize_yes_no(value)) that lowercases the
input, checks membership in YES_NO_MAP, returns the mapped boolean on success,
and raises SAPCliError with a clear message on failure; then replace the direct
YES_NO_MAP[...] lookups in the cloud_dev and key_user_apps branches to call that
helper. Ensure the helper is used consistently wherever YES_NO_MAP indexing
occurs.
In `@test/unit/fixtures_adt_apirelease.py`:
- Line 1: Rename the fixture module to use the test_ prefix and match the tested
module suffix (for example rename fixtures_adt_apirelease.py to
test_sap_adt_apirelease_fixtures.py or move the fixtures into the test module),
then update the import in test/unit/test_sap_adt_apirelease.py to import the new
module name (adjust the import at the top of that file accordingly) so the
fixture module follows the "test_" naming convention and the test import
resolves.
In `@test/unit/test_sap_cli_apirelease.py`:
- Around line 155-156: The wrapped call to _parse in the test (e.g., the args =
_parse(...) statement) uses over-indented continuation lines causing Flake8
E127; fix by aligning each continuation line directly under the opening
parenthesis of the _parse call or place one argument per line so the indentation
is consistent (apply the same pattern to all other occurrences listed — the
other _parse/assert calls around lines 176-177, 194-195, 213-214, 234-235,
253-254, 272-273, 283-284, 298-299, 304-305, 315-316, 326-327, 342-343, 359-360,
374-375, 389-390, 401-402), ensuring each wrapped line lines up with the first
character after the opening parenthesis.
- Around line 16-23: Remove the unused OBJECT_URI symbol from the
fixtures_adt_apirelease import list in the test module so Flake8 stops flagging
it; locate the import statement that currently pulls in OBJECT_URI along with
API_RELEASE_RESPONSE_XML, VALIDATION_RESPONSE_INFO_XML,
VALIDATION_RESPONSE_ERROR_XML, VALIDATION_RESPONSE_WARNING_XML, and
SET_API_RELEASE_RESPONSE_XML, and delete OBJECT_URI from that tuple (only re-add
it if a later assertion truly requires it).
---
Nitpick comments:
In `@sap/adt/apirelease.py`:
- Line 170: Remove the duplicated, unused constant VALID_CONTRACTS from the
module: delete the line defining VALID_CONTRACTS = ('c0', 'c1', 'c2', 'c3',
'c4') and any references to it; rely on the authoritative ContractKey definition
instead (ensure no other code in this file references VALID_CONTRACTS and update
tests or callers to use ContractKey if needed).
In `@test/unit/test_sap_cli_apirelease.py`:
- Around line 16-71: The test currently declares inline XML constants
(API_RELEASE_NO_CONTRACTS_XML, API_RELEASE_READONLY_XML, NO_BEHAVIOUR_XML) while
also importing fixtures from fixtures_adt_apirelease (which lacks the required
test_ prefix); move those inline XML strings into a new same-directory fixture
module named with the test_ prefix (e.g., test_fixtures_adt_apirelease.py),
place all related constants (API_RELEASE_NO_CONTRACTS_XML,
API_RELEASE_READONLY_XML, NO_BEHAVIOUR_XML and the previously imported
OBJECT_URI, API_RELEASE_RESPONSE_XML, etc.) into that module, then update the
test file to import these symbols from the new test_fixtures module and remove
the inline declarations; ensure the new module lives in test/unit/ and uses the
exact symbol names used by the test so no other code changes are required.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dfec785f-cb49-49be-8171-2315eacf5c9c
📒 Files selected for processing (9)
doc/architecture.mdsap/adt/__init__.pysap/adt/annotations.pysap/adt/apirelease.pysap/cli/apirelease.pysap/cli/datadefinition.pytest/unit/fixtures_adt_apirelease.pytest/unit/test_sap_adt_apirelease.pytest/unit/test_sap_cli_apirelease.py
| @@ -0,0 +1,163 @@ | |||
| """Fixtures for sap.adt.apirelease tests""" | |||
There was a problem hiding this comment.
Fixture filename violates naming convention.
This file should use the test_ prefix (e.g., test_sap_adt_apirelease_fixtures.py or inline the fixtures into the existing test module). The import in test/unit/test_sap_adt_apirelease.py line 10 would need updating accordingly.
As per coding guidelines: "Test fixtures should be stored as module files with the prefix test_" and "Test fixture file names should have suffixes matching the tested module in the same way as test files".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/unit/fixtures_adt_apirelease.py` at line 1, Rename the fixture module to
use the test_ prefix and match the tested module suffix (for example rename
fixtures_adt_apirelease.py to test_sap_adt_apirelease_fixtures.py or move the
fixtures into the test module), then update the import in
test/unit/test_sap_adt_apirelease.py to import the new module name (adjust the
import at the top of that file accordingly) so the fixture module follows the
"test_" naming convention and the test import resolves.
Comment on lines
+155
to
+156
| args = _parse(group, 'apistate', 'set', 'c1', 'I_STATISTICALKEYFIGURECAT', | ||
| '--state', 'Released', '--corrnr', 'C50K000061') |
There was a problem hiding this comment.
Fix the continuation indentation reported by Flake8.
These wrapped argument/assertion lines are over-indented per E127 and can fail CI. Align continuation lines consistently under the opening call or use one argument per line.
🧹 Example fix pattern
- args = _parse(group, 'apistate', 'set', 'c1', 'I_STATISTICALKEYFIGURECAT',
- '--state', 'Released', '--corrnr', 'C50K000061')
+ args = _parse(
+ group, 'apistate', 'set', 'c1', 'I_STATISTICALKEYFIGURECAT',
+ '--state', 'Released', '--corrnr', 'C50K000061')Apply the same wrapping style to the other Flake8-reported continuations.
Also applies to: 176-177, 194-195, 213-214, 234-235, 253-254, 272-273, 283-284, 298-299, 304-305, 315-316, 326-327, 342-343, 359-360, 374-375, 389-390, 401-402
🧰 Tools
🪛 Flake8 (7.3.0)
[error] 156-156: continuation line over-indented for visual indent
(E127)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/unit/test_sap_cli_apirelease.py` around lines 155 - 156, The wrapped
call to _parse in the test (e.g., the args = _parse(...) statement) uses
over-indented continuation lines causing Flake8 E127; fix by aligning each
continuation line directly under the opening parenthesis of the _parse call or
place one argument per line so the indentation is consistent (apply the same
pattern to all other occurrences listed — the other _parse/assert calls around
lines 176-177, 194-195, 213-214, 234-235, 253-254, 272-273, 283-284, 298-299,
304-305, 315-316, 326-327, 342-343, 359-360, 374-375, 389-390, 401-402),
ensuring each wrapped line lines up with the first character after the opening
parenthesis.
filak-sap
force-pushed
the
initial_ddl_apireleases
branch
from
f8b7cfd to
1f86a69
Compare
Add mappings based on captured communication for reading, adding and updating API State Contracts for CDS views (DDLS). The delete operation has not been tested yet. Also not all API Releases features are supported because of lack of test data and test documentation. I do not use the ABAP source codes for extracting of the logic because I am afraid of violating licenses.
Plenty of objects support API Releases and we do not want to have a special command group for "sapcli apireleases" but we want to make apireleases part of the regular command group. Therefor this mixin.
Just use the mixin exposing API Releases for an object based command group. Perhaps we can extend the mixin to be aware of which Contracts are supported and which are not because CDS should support only C0, C1, and C2. The Contracts C3 and C4 should never be set for CDS.
filak-sap
force-pushed
the
initial_ddl_apireleases
branch
from
1f86a69 to
abb33c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
test/unit/test_sap_cli_apirelease.py (1)
156-157:⚠️ Potential issue | 🟡 MinorFix the Flake8 E127 continuation indentation.
These wrapped continuation lines are still over-indented and will fail linting. Use a consistent multiline call style.
🧹 Example fix pattern
- args = _parse(group, 'apistate', 'set', 'c1', 'I_STATISTICALKEYFIGURECAT', - '--state', 'Released', '--corrnr', 'C50K000061') + args = _parse( + group, 'apistate', 'set', 'c1', 'I_STATISTICALKEYFIGURECAT', + '--state', 'Released', '--corrnr', 'C50K000061')Also applies to: 177-178, 195-196, 214-215, 235-236, 254-255, 273-274, 284-285, 299-300, 305-306, 316-317, 327-328, 343-344, 360-361, 375-376, 390-391, 402-403
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/test_sap_cli_apirelease.py` around lines 156 - 157, The test call to _parse in test_sap_cli_apirelease.py is over-indented for a continued multiline argument list and triggers Flake8 E127; reformat all such wrapped _parse(...) calls (e.g., the instance at lines calling _parse(group, 'apistate', 'set', 'c1', 'I_STATISTICALKEYFIGURECAT', '--state', 'Released', '--corrnr', 'C50K000061')) to use a consistent continuation style — either place the first argument on the same line as _parse( and align all subsequent arguments under the first argument or use a hanging indent with a single level (4 spaces) inside the parentheses — and apply the same change to the other occurrences noted (lines around the other _parse calls).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@doc/commands/ddl.md`:
- Around line 51-64: Update the fenced sample output block in the
doc/commands/ddl.md example to include a language tag "text" (i.e., change the
opening ``` to ```text) so markdownlint MD040 is satisfied; locate the "Sample
output:" fenced code block showing the contract lines and replace its fence with
```text to mark it as plain CLI output.
In `@sap/adt/apirelease.py`:
- Around line 194-213: The copy_contract function currently copies only a subset
of fields (contract, use_in_key_user_apps, use_in_sap_cloud_platform, comment,
and status), which can unintentionally reset other existing Contract fields;
modify copy_contract (in apirelease.py) to clone all fields from the existing
Contract when present (rather than assigning only selected attributes) — e.g.,
perform a shallow/deep copy of the existing Contract object into target
(preserving fields like featureToggle, createAuthValues, successor data, state
transitions, transportObject, etc.), then update only the necessary fields
(status handling via ContractStatus) without reinitializing unspecified fields
so the serialized PUT payload does not overwrite defaults.
In `@sap/cli/apirelease.py`:
- Around line 207-210: Add a short explanatory comment next to the try/except
around the input call (the block using input('Continue despite warnings? [y/N]
') that sets answer = '' on EOFError) stating that EOFError is intentionally
swallowed because non-interactive/EOF input should be treated as a default "no"
(do not continue), so the fallback answer = '' implements the safe default; keep
the exception handling but document the rationale and intended behavior for
future readers.
---
Duplicate comments:
In `@test/unit/test_sap_cli_apirelease.py`:
- Around line 156-157: The test call to _parse in test_sap_cli_apirelease.py is
over-indented for a continued multiline argument list and triggers Flake8 E127;
reformat all such wrapped _parse(...) calls (e.g., the instance at lines calling
_parse(group, 'apistate', 'set', 'c1', 'I_STATISTICALKEYFIGURECAT', '--state',
'Released', '--corrnr', 'C50K000061')) to use a consistent continuation style —
either place the first argument on the same line as _parse( and align all
subsequent arguments under the first argument or use a hanging indent with a
single level (4 spaces) inside the parentheses — and apply the same change to
the other occurrences noted (lines around the other _parse calls).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 51fc676d-3b41-40b7-9050-246a0cff9282
📒 Files selected for processing (10)
doc/architecture.mddoc/commands/ddl.mdsap/adt/__init__.pysap/adt/annotations.pysap/adt/apirelease.pysap/cli/apirelease.pysap/cli/datadefinition.pytest/unit/fixtures_adt_apirelease.pytest/unit/test_sap_adt_apirelease.pytest/unit/test_sap_cli_apirelease.py
✅ Files skipped from review due to trivial changes (4)
- doc/architecture.md
- sap/adt/annotations.py
- test/unit/fixtures_adt_apirelease.py
- test/unit/test_sap_adt_apirelease.py
🚧 Files skipped from review as they are similar to previous changes (2)
- sap/cli/datadefinition.py
- sap/adt/init.py
Comment on lines
+51
to
+64
| Sample output: | ||
|
|
||
| ``` | ||
| Extended (Contract C0): not set | ||
| Use System-Internally (Contract C1): | ||
| Release State: Released | ||
| Local Comment: | ||
| Use in Cloud Development: No | ||
| Use in Key User Apps: Yes | ||
| Authorization Default Value: disabled | ||
| Use as Remote API (Contract C2): not set | ||
| Contract C3: not set | ||
| Contract C4: not set | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the sample output fence.
markdownlint flags this as MD040. Use text for plain CLI output.
🧹 Proposed fix
Sample output:
-```
+```text
Extended (Contract C0): not set
Use System-Internally (Contract C1):
Release State: Released📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Sample output: | |
| ``` | |
| Extended (Contract C0): not set | |
| Use System-Internally (Contract C1): | |
| Release State: Released | |
| Local Comment: | |
| Use in Cloud Development: No | |
| Use in Key User Apps: Yes | |
| Authorization Default Value: disabled | |
| Use as Remote API (Contract C2): not set | |
| Contract C3: not set | |
| Contract C4: not set | |
| ``` | |
| Sample output: | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@doc/commands/ddl.md` around lines 51 - 64, Update the fenced sample output
block in the doc/commands/ddl.md example to include a language tag "text" (i.e.,
change the opening ``` to ```text) so markdownlint MD040 is satisfied; locate
the "Sample output:" fenced code block showing the contract lines and replace
its fence with ```text to mark it as plain CLI output.
Comment on lines
+194
to
+213
| def copy_contract(self, contract_key: ContractKey) -> Contract: | ||
| """Create a copy of the existing contract for the given key, or a blank one if not set""" | ||
|
|
||
| existing = self.get_contract(contract_key) | ||
| target = Contract() | ||
|
|
||
| if existing is not None and existing.contract is not None: | ||
| target.contract = existing.contract | ||
| target.use_in_key_user_apps = existing.use_in_key_user_apps | ||
| target.use_in_sap_cloud_platform = existing.use_in_sap_cloud_platform | ||
| target.comment = existing.comment | ||
| if existing.status is not None: | ||
| target.status = ContractStatus() | ||
| target.status.state = existing.status.state | ||
| target.status.state_description = existing.status.state_description | ||
| else: | ||
| target.contract = contract_key.value.upper() | ||
| target.status = ContractStatus() | ||
|
|
||
| return target |
There was a problem hiding this comment.
Preserve all existing contract fields when building an update payload.
copy_contract() copies only contract, use flags, comment, and status. Because several Contract descriptors have defaults, the serialized PUT payload can unintentionally reset existing fields such as featureToggle, createAuthValues, successor data, state transitions, or transportObject.
🔧 Proposed fix
+from copy import deepcopy
from typing import Optional
from enum import Enum def copy_contract(self, contract_key: ContractKey) -> Contract:
"""Create a copy of the existing contract for the given key, or a blank one if not set"""
existing = self.get_contract(contract_key)
- target = Contract()
if existing is not None and existing.contract is not None:
- target.contract = existing.contract
- target.use_in_key_user_apps = existing.use_in_key_user_apps
- target.use_in_sap_cloud_platform = existing.use_in_sap_cloud_platform
- target.comment = existing.comment
- if existing.status is not None:
- target.status = ContractStatus()
- target.status.state = existing.status.state
- target.status.state_description = existing.status.state_description
+ target = deepcopy(existing)
else:
+ target = Contract()
target.contract = contract_key.value.upper()
target.status = ContractStatus()
return target🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sap/adt/apirelease.py` around lines 194 - 213, The copy_contract function
currently copies only a subset of fields (contract, use_in_key_user_apps,
use_in_sap_cloud_platform, comment, and status), which can unintentionally reset
other existing Contract fields; modify copy_contract (in apirelease.py) to clone
all fields from the existing Contract when present (rather than assigning only
selected attributes) — e.g., perform a shallow/deep copy of the existing
Contract object into target (preserving fields like featureToggle,
createAuthValues, successor data, state transitions, transportObject, etc.),
then update only the necessary fields (status handling via ContractStatus)
without reinitializing unspecified fields so the serialized PUT payload does not
overwrite defaults.
Comment on lines
+207
to
+210
| try: | ||
| answer = input('Continue despite warnings? [y/N] ') | ||
| except EOFError: | ||
| answer = '' |
There was a problem hiding this comment.
Document why EOFError is intentionally swallowed.
The fallback is sensible, but add a short comment so future readers know EOF is treated as “do not continue”.
🧹 Proposed fix
try:
answer = input('Continue despite warnings? [y/N] ')
except EOFError:
+ # Treat EOF as a negative response when stdin closes during confirmation.
answer = ''As per coding guidelines: “Avoid silently swallowing caught exceptions in Python code. If exception swallowing is necessary, add a comment explaining why it is needed”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sap/cli/apirelease.py` around lines 207 - 210, Add a short explanatory
comment next to the try/except around the input call (the block using
input('Continue despite warnings? [y/N] ') that sets answer = '' on EOFError)
stating that EOFError is intentionally swallowed because non-interactive/EOF
input should be treated as a default "no" (do not continue), so the fallback
answer = '' implements the safe default; keep the exception handling but
document the rationale and intended behavior for future readers.
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
2 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.
No description provided.