◐ Shell
clean mode source ↗

fix: Add conn.commit() to Postgresonline_write_batch.online_write_batch by job-almekinders · Pull Request #3904 · feast-dev/feast

Merged

HaoXuAI

merged 1 commit into

Feb 9, 2024

Conversation

@job-almekinders

What this PR does / why we need it:

This PR ensures that the materialization step in the PostgreSQLOnlineStore succeeds and thus the data is pushed to the online store. It does so by running conn.commit() after the insert SQL statement has ran.

Which issue(s) this PR fixes:
Fixes #3903

@job-almekinders

I don't understand why the DCO is failing; I've signed my commit, and it shows that the commit is 'Verified' here in the PR.

@tokoko

Signed-off-by: Job Almekinders <55230856+job-almekinders@users.noreply.github.com>

@job-almekinders

@tokoko Thank you for the elaboration. I pushed the changes now with the correct signature 👍

@tokoko

HaoXuAI

Choose a reason for hiding this comment

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

LGTM!

@job-almekinders

@HaoXuAI Thanks for taking the time to review :)

The CI check integration-test-python (3.8, ubuntu-latest) seems to be stuck. At least, the state hasn't changed for a few days. Do you perhaps know how we might be able to resolve this?

@HaoXuAI

@HaoXuAI Thanks for taking the time to review :)

The CI check integration-test-python (3.8, ubuntu-latest) seems to be stuck. At least, the state hasn't changed for a few days. Do you perhaps know how we might be able to resolve this?

This seems to be a staled test and got stuck. Can you please rebase master and try again?

@tokoko

@HaoXuAI pretty sure it just needs ok-to-test label in order for cloud integration tests to be run.

@HaoXuAI

@HaoXuAI pretty sure it just needs ok-to-test label in order for cloud integration tests to be run.

Ah ok, just added. 🤞

tqtensor pushed a commit to tqtensor/feast that referenced this pull request

Mar 11, 2024

Labels