PR#3364: Catch koji.AuthError and bail out - koji

koji

#3364 Catch koji.AuthError and bail out

Merged 2 years ago by tkopecek. Opened 2 years ago by alexi.

alexi/koji autherror into master

Alex Iribarren • 2 years ago

99c4040

builder/kojid

file modified

		`@@ -159,6 +159,10 @@`
		`logger.error('Session expired')`
		`exit_code = 1`
		`break`
		`+ except koji.AuthError:`
		`+ logger.error('Authentication error')`
		`+ exit_code = 1`
		`+ break`
		`except koji.RetryError:`
		`raise`
		`except Exception:`

util/kojira

file modified

		`@@ -1159,6 +1159,9 @@`
		`except koji.AuthExpired:`
		`logger.warning("Session expired")`
		`break`
		`+ except koji.AuthError:`
		`+ logger.warning("Authentication error")`
		`+ break`
		`except SystemExit:`
		`logger.warning("Shutting down")`
		`break`

alexi commented 2 years ago

AuthError exceptions were not being caught and dealt with, this PR fixes that.

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

2 years ago

mikem commented 2 years ago

I'm not sure this really deals with them, but they are generally indicative of a non-recoverable error, so probably better to exit than to keep trying.

Granted, both of these have Restart=on-failure in their systemd service config, so it ends up being a heavier-handed form of retry. In the case where a session has been expired, this may well resolve the issue. For other types of AuthError faults (disabled user, access issues, bad client code), a full restart will probably not help.

mikem commented 2 years ago

Longer term, we should probably try to have a cleaner reauth mechanism for expired sessions, but this simple change is fine for now.

Metadata Update from @tkopecek:
- Pull-request untagged with: testing-ready
- Pull-request tagged with: no_qe

2 years ago