Skip to content

Performance and other improvements#33

Merged
jpodivin merged 8 commits into
fedora-copr:mainfrom
jpodivin:perf
Apr 8, 2026
Merged

Performance and other improvements#33
jpodivin merged 8 commits into
fedora-copr:mainfrom
jpodivin:perf

Conversation

@jpodivin

@jpodivin jpodivin commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Upon examination, it turned out that we are mostly right about the worker number. In fact, even 4 may be an overkill, since we are I/O bound. I have at least parametrized it, so we can adjust the value in production.

Further changes:

  • Timestamps are now handled more consistently, as a datetime object until use
  • Http client is initialized once
  • Tasks are tracked and their state verified upon exit
  • Server has a lifespan manager, disposing of tasks gracefully
  • The logger initialization has been normalized
  • We are now mocking the LD_URL in tests

jpodivin added 7 commits April 8, 2026 11:31
Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
Signed-off-by: Jiri Podivin <jpodivin@redhat.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a global AsyncClient for HTTP requests and implements a lifespan manager to ensure background tasks are completed during application shutdown. It also refactors the handling of analysis start times to use datetime objects instead of strings and improves task error reporting via Sentry. A review comment suggests explicitly closing the http_client within the lifespan manager to ensure all resources are properly released.

Comment thread src/logdetective_packit/main.py

@Jany26 Jany26 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if we should add some config file, as the number of env variables keeps increasing 😅

Comment thread src/logdetective_packit/main.py Outdated
Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
@jpodivin jpodivin merged commit d6417c6 into fedora-copr:main Apr 8, 2026
3 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.

2 participants