Skip to content

Ensure configure-central task is indempotent#14

Open
mgoerens wants to merge 1 commit into
opdev:mainfrom
mgoerens:indempotent_configure_central
Open

Ensure configure-central task is indempotent#14
mgoerens wants to merge 1 commit into
opdev:mainfrom
mgoerens:indempotent_configure_central

Conversation

@mgoerens

Copy link
Copy Markdown
Collaborator

The task would previously fail if the image integration was already configured.

@exe-prow-github-app exe-prow-github-app Bot requested a review from lslavkov June 15, 2026 15:39
@exe-prow-github-app

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mgoerens

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@exe-prow-github-app exe-prow-github-app Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 15, 2026
Comment thread rhacs/tasks/configure-central.yaml Outdated
message = None
try:
message = response.json().get("message")
except requests.exceptions.JSONDecodeError:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This only ensures that JSON has failed. Would ass as well for AttributeError.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I believe we're good here, unless I'm missing something that could raise AttributeError:

  • Response.json() only can raise requests.exceptions.JSONDecodeError.
  • dict.get() never raises exceptions, by default it returns None if the key isn't present in the dictionary.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

response.json() assumes that it is returning dict. Error could be like this
AttributeError: 'list' object has no attribute 'get' which can create unexpected problem. Based on Gemini there is alternative:

try:
    data = response.json()
    message = data.get("message") if isinstance(data, dict) else None
except requests.exceptions.JSONDecodeError:
    pass

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I see, good catch. I went to your initial suggestion and also switched to using suppress as I think this fits nicely for this use case.

Comment thread rhacs/tasks/configure-central.yaml Outdated
@mgoerens mgoerens force-pushed the indempotent_configure_central branch from ae236f5 to ff94b98 Compare June 17, 2026 11:51
@mgoerens mgoerens requested a review from lslavkov June 17, 2026 11:53
The task would previously fail if the image integration was already
configured.

Signed-off-by: Matthias Goerens <mgoerens@redhat.com>
@mgoerens mgoerens force-pushed the indempotent_configure_central branch from ff94b98 to 9731919 Compare June 17, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants