Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Remove more usages of cursor_to_dict #16551

Merged
merged 12 commits into from
Oct 26, 2023
Merged

Conversation

clokep
Copy link
Member

@clokep clokep commented Oct 25, 2023

More of #16431, this targets a bunch of individual usages, but also targets the usages of execute which can take a callable to map the results. In actuality it always passes cursor_to_dict.

This can be split further, but isn't too large and should be reviewable commit-by-commit.

@clokep
Copy link
Member Author

clokep commented Oct 25, 2023

After this there's < 10 usages remaining.

@clokep clokep force-pushed the clokep/axe-cursor-to-dict-6 branch from c73d1d6 to 9937cdf Compare October 26, 2023 11:56
@clokep clokep marked this pull request as ready for review October 26, 2023 12:20
@clokep clokep requested a review from a team as a code owner October 26, 2023 12:20
# We need to pass execute a dummy function to handle the txn's result otherwise
# it tries to call fetchall() on it and fails because there's no result to fetch.
await self.db_pool.execute(
await self.db_pool.runInteraction(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the point that we should use runInteraction when we don't expect---or don't care---about the result set?

Copy link
Contributor

Choose a reason for hiding this comment

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

and the only place where we used a decoder before was where we were trying to ignore the result set!?!

Copy link
Member Author

Choose a reason for hiding this comment

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

Is the point that we should use runInteraction when we don't expect---or don't care---about the result set?

execute(...) automatically calls fetchall() (or iterates the txn if you use cursor_to_dict. You could do hacky things like this to not iterate the txn and just leave it, but...I think this is as clear.

and the only place where we used a decoder before was where we were trying to ignore the result set!?!

Or we passed cursor_to_dict in many many places, which are all gone.

Copy link
Contributor

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@clokep clokep merged commit 679c691 into develop Oct 26, 2023
37 of 40 checks passed
@clokep clokep deleted the clokep/axe-cursor-to-dict-6 branch October 26, 2023 19:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants