Skip to content

Cleanup: get_running() methods return query#3018

Open
m-blaha wants to merge 1 commit intopackit:mainfrom
m-blaha:return-query
Open

Cleanup: get_running() methods return query#3018
m-blaha wants to merge 1 commit intopackit:mainfrom
m-blaha:return-query

Conversation

@m-blaha
Copy link
Member

@m-blaha m-blaha commented Feb 24, 2026

Makes *.get_running() methods consistent with other model methods that
already return a query. Returns model instances directly instead of
single-element tuples, removing awkward (obj,) unpacking in
all callers.

Resolves: #3004

@centosinfra-prod-github-app
Copy link
Contributor

@m-blaha
Copy link
Member Author

m-blaha commented Feb 24, 2026

/gemini review

Copy link
Contributor

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

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 is a nice cleanup that refactors the get_running() methods in the database models. By changing them to return model instances directly instead of single-element tuples, the code is now more consistent with other model methods and easier to use, as it removes the need for tuple unpacking in the calling code. The changes are applied consistently across all affected models, helper functions, and tests. This is a good improvement to the codebase, and I have no further comments.

Copy link
Contributor

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

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 is a nice cleanup that makes the get_running() methods in the models consistent with the rest of the codebase by returning model instances directly instead of tuples. This simplifies the calling code by removing the need for tuple unpacking. The changes are well-executed across all affected files, including models, helpers, and tests. I've found one minor issue with a docstring that I've commented on. Otherwise, great work!

Copy link
Member

@nforro nforro left a comment

Choose a reason for hiding this comment

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

LGTM, but let's get an opinion from @mfocko who introduced the select in #2774.

Makes *.get_running() methods consistent with other model methods that
already return a query. Returns model instances directly instead of
single-element tuples, removing awkward (obj,) unpacking in
all callers.

Resolves: packit#3004
Signed-off-by: Marek Blaha <mblaha@redhat.com>
@centosinfra-prod-github-app
Copy link
Contributor

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

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

Replace session.execute(select(...)) with session.query() in get_running() methods

2 participants