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

Update Prostgres Persistence #49

Merged
merged 2 commits into from
May 24, 2022
Merged

Conversation

Bibo-Joshi
Copy link
Member

I started out with just updating to v20, but ended up also

  • refactoring the internals to better leverage the parent class DictPersistence and
  • adding support for callback_data

Unfortunately I'm not familiar for postgres (& databases in general), so I didn't try to actually test this with a real database (instead of just mocking as in the tests). If would be great if someone could cover that part :) CCing @starry69 for completeness 🙆‍♂️

It's not necessary that this is merged into v20-updates before #47 is merged. Just chose that as target branch so that the differences to main are more clearly visible.

@starry-shivam
Copy link
Member

Hi, really thanks for taking your time to update this module, changes look good overall, I'll test it with real db in the evening and inform you.

@starry-shivam
Copy link
Member

So i tested with actual db, apart from conversation related data everything seemed to work fine, was getting:

/data/data/com.termux/files/usr/lib/python3.10/site-packages/telegram/ext/_dictpersistence.py", line 170, in init
    raise TypeError(
TypeError: Unable to deserialize conversations_json. Not valid JSON

When loading conversation data, here is how data looks like in db for reference:

{"chat_data": "{\"894380120\": {}}", "user_da
ta": "{\"894380120\": {}}", "bot_data": "{}",
"conversations": "null", "callback_data": "[[]
, {}]"}
(1 row)

I think here in line 474 of dictpersistence when it does json.loads("null") it evaluates to None and ofcourse NoneType object don't have any .items() attribute it raises this error at line 169 of dictpersistence. BTW adding conversations_json = data.get("conversations", "{}") at line 85 seems to fix the issue. Also i think _key_maper() function is not used anywhere now, so it's better to remove that.

@Bibo-Joshi
Copy link
Member Author

Thanks for testing 💪 The fix makes sense - just pushed it. If you could double check with the fix, that would be awesome :)

@starry-shivam
Copy link
Member

Sure! Will do it in morning.

Copy link
Member

@starry-shivam starry-shivam left a comment

Choose a reason for hiding this comment

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

Tested again, everything seems to work fine.

@Bibo-Joshi Bibo-Joshi merged commit 86f20ab into v20-updates May 24, 2022
@Bibo-Joshi Bibo-Joshi deleted the v20-postgres-persistence branch May 24, 2022 05:46
@Bibo-Joshi
Copy link
Member Author

Thanks again for testing :)

@github-actions github-actions bot locked and limited conversation to collaborators May 25, 2022
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.

None yet

2 participants