-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Postgres trigger #9218
base: main
Are you sure you want to change the base?
Postgres trigger #9218
Conversation
# Conflicts: # mindsdb/integrations/handlers/postgres_handler/postgres_handler.py
def subscribe(self, stop_event, callback, table_name, columns=None, **kwargs): | ||
|
||
# psycopg2 is used | ||
import psycopg2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not to use psycopg
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to psycopg
|
||
while conn.notifies: | ||
process_event(conn.notifies.pop()) | ||
conn.poll() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it required? will it clear current notifies
list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was updated after switching to psycopg
conn.poll() | ||
|
||
while conn.notifies: | ||
process_event(conn.notifies.pop()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this case processing order if LIFO. Do we need FIFO? If so, then need to add 0 to .pop(0)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was updated after switching to psycopg
# exit trigger | ||
return | ||
|
||
conn.add_notify_handler(process_event) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it required to add handler in each iteration?
|
||
conn.add_notify_handler(process_event) | ||
# trigger getting updates | ||
conn.execute("SELECT 1").fetchone() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spam db with query each 0.5s looks badly. Let put here comment with reminder to rewrite it to generator when psycopg 3.2.2 will be released (they added timeout arg there)
Description
Added
subscribe
method to postgres handler.How it works:
Limitations:
Example
Demo
https://www.loom.com/share/8b27d9c467e6471e89ab22121daa7b85
Fixes #issue_number
Type of change
Verification Process
To ensure the changes are working as expected:
Additional Media:
Checklist: