Skip to content

feat(core): add KeyAuth.close() to stop the token refresh thread - #10303

Open
mittalpk wants to merge 3 commits into
stackitcloud:mainfrom
mittalpk:fix/keyauth-close
Open

mittalpk wants to merge 3 commits into
stackitcloud:mainfrom
mittalpk:fix/keyauth-close

Conversation

@mittalpk

Copy link
Copy Markdown

Description

relates to #10253

KeyAuth started a daemon refresh thread that looped forever with no way to stop it, so every discarded instance (for example after key rotation in a long-running process) kept a thread alive that kept requesting tokens. This adds a public, idempotent close(), as proposed in the issue: the loop now waits on a stop event, and close() sets it, shuts down the refresh executor, unregisters the atexit hook and joins the thread.

I bumped core to v0.4.0 since this adds a public method. v0.3.1 is not released yet, so I'm happy to fold this into it if you prefer.

Checklist

  • Issue was linked above
  • No generated code was adjusted manually (check comments in file header)
  • Changelogs and versioning
    • Changelog in root directory was adjusted (see here)
    • Changelog of the service(s) was adjusted (see e.g. here)
    • pyproject.toml of the service(s) was adjusted (see e.g. here)
  • Examples were added / adjusted (see examples/ directory)
  • Unit tests got implemented or updated
  • Unit tests are passing: make test (will be checked by CI)
  • No linter issues: make lint (will be checked by CI)

Each KeyAuth started a daemon thread that refreshed tokens forever, with
no way to stop it, so a discarded instance kept running and requesting
tokens. close() stops the loop, shuts down the refresh executor and
removes the atexit hook. It is safe to call more than once.
@cgoetz-inovex

Copy link
Copy Markdown
Contributor

Hi @mittalpk,
thanks for your contribution, CI is currently failing with:

Lock file for alb is out of date. Run 'make update-dependencies'.

@mittalpk

Copy link
Copy Markdown
Author

Thanks, the core version bump left the service lock files stale. I ran make update-dependencies and pushed the relocked files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants