Skip to content

Conversation

@amcmahon-rh
Copy link
Contributor

We have seen several cases where Pub tasks fail because the worker gets a permission denied error, immediately after successfully logging in. We believe that these are false negatives, which can be resolved by a simple retry.

@rbikar
Copy link
Member

rbikar commented Mar 27, 2025

@amcmahon-rh can you please add/update some test that covers this part?

We have seen several cases where Pub tasks fail because the worker gets
a permission denied error, immediately after successfully logging in. We
believe that these are false negatives, which can be resolved by a
simple retry.
@amcmahon-rh amcmahon-rh changed the title Add PermissionDenied exception to retry decorator Add XMLRPC Fault exception to retry decorator Mar 28, 2025
Copy link
Contributor

@kdudka kdudka left a comment

Choose a reason for hiding this comment

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

When xmlrpclib.ProtocolError was added in the list via #173, it caused a regression in OSH, reported as #270, which has not yet been fixed. I am afraid that adding xmlrpclib.Fault may have similar undesired side effects.

@amcmahon-rh
Copy link
Contributor Author

@kdudka would you be ok with this change if the xmlrpclib.Fault handling was restricted to permission denied errors specifically?

Something like:

                except (socket.error, socket.herror, socket.gaierror, socket.timeout,
                        httplib.CannotSendRequest, xmlrpclib.ProtocolError,
                        xmlrpclib.Fault) as ex:
                    if type(ex) is xmlrpclib.Fault and \
                            not "PermissionDenied" in ex.faultString:
                        raise ex

@kdudka
Copy link
Contributor

kdudka commented Mar 31, 2025

Yes, making the condition to retry the request more specific sounds like a good idea.

@amcmahon-rh amcmahon-rh merged commit 79c8169 into release-engineering:master Apr 1, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants