Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated db.py to comply to sqlalchemy 1.4 #85

Merged
merged 1 commit into from
May 16, 2022

Conversation

flochir
Copy link
Contributor

@flochir flochir commented Feb 7, 2022

Fixed Issue #79 by switching table.count() to equivalent calls to func.count().

Description

Ran into Issue #79 trying to install and run gourmand from AUR on Arch Linux.
I switched to func.count() following the sqlalchemy 1.4 API doc. As func.count() needs an attribute to run on, I chose to use the primary key of the table for that (usually just an int and present). Another candidate would have been table.c[0].

How Has This Been Tested?

Looked at the generated Query structure in fetch_len():

...
        criteria={'id':1628}
        print("With criteria: ", select(
                [func.count(list(table.primary_key.columns)[0])])\
                    .where(*make_simple_select_arg(criteria,table)).compile())

        print("Without criteria: ", select(
            [func.count(list(table.primary_key.columns)[0])]).compile())
...

yields:

With criteria:  SELECT count(keylookup.id) AS count_1 
FROM keylookup 
WHERE keylookup.id = ?
Without criteria:  SELECT count(keylookup.id) AS count_1 
FROM keylookup

... which I guess is what was the purpose of that. The code returns plausible values for the count() queries ("1" with the criterium above, "1638" without).

Also the fixed code works fine so far with sqlalchemy 1.4, however I could not find out how to get the application to actually call fetch_len() with a set of criteria. Also I didn't dig deeper into esp. make_simple_select_arg(), and this is just a quick fix.
Please verify.

Style (of the changed bits) checks out with pylint / PEP8.

Types of changes

[x] Updated use of library, removed deprecated function

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.

@flochir
Copy link
Contributor Author

flochir commented Feb 7, 2022

Oh boy, just went into fixing and actually missed #80 by @marillat, sorry!

@cydanil
Copy link
Contributor

cydanil commented Feb 7, 2022

Thank you for this fix nontheless!

I will evaluate both solutions, and choose one 🙂

It seems that the CI is failing for problems unrelated to your changes. I will have a look, and maybe I'll ask you to rebase.

Cheers,
Cyril

@cydanil
Copy link
Contributor

cydanil commented May 16, 2022

Thank you again for your contribution, I'm going ahead with your fix. Thanks a lot!

@cydanil cydanil merged commit 0d23126 into GourmandRecipeManager:main May 16, 2022
@flochir
Copy link
Contributor Author

flochir commented May 18, 2022

You're welcome!

winshipvarner referenced this pull request in winshipvarner/gourmand Oct 3, 2024
…ugin. Field editor work: expanded the treeview and added a header that allows for sorting alphabetically. Also applied patch to db.py to comply with sqlalchemy 1.4
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