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

feat: Add event trigger to handle job cleanup on table drop in vectorize schema #178

Merged

Conversation

akhilender-bongirwar
Copy link
Contributor

fixes #148
/claim #148

@akhilender-bongirwar
Copy link
Contributor Author

@ChuckHend @jasonmp85, please review. I would be happy to incorporate any suggestions you have : )

@varshith257
Copy link

@akhilender-bongirwar another duplicate efforts with #164

PS: It's getting reviewed though

I got occupied with other works recently and didn't made time to push to endline and complete PR. Anyhow will complete it now.

@akhilender-bongirwar
Copy link
Contributor Author

akhilender-bongirwar commented Oct 29, 2024

@akhilender-bongirwar another duplicate efforts with #164

PS: It's getting reviewed though

I got occupied with other works recently and didn't made time to push to endline and complete PR. Anyhow will complete it now.

@varshith257 I saw your PR was inactive for 5 days and I had a different idea for resolving the issue.

@akhilender-bongirwar akhilender-bongirwar force-pushed the feat/event-trigger-for-drop-table branch from 0dfa9b8 to 2838e75 Compare October 31, 2024 03:42
@akhilender-bongirwar
Copy link
Contributor Author

@ChuckHend, could you also review my implementation?

I have a question: are all the integration tests ignored by default? If so, how are we writing and running tests to verify the changes?

@akhilender-bongirwar akhilender-bongirwar force-pushed the feat/event-trigger-for-drop-table branch from 7b13509 to 466b34b Compare November 1, 2024 15:28
@ChuckHend
Copy link
Member

All of the CI was paused since this PR is from a branch outside the tembo-io org. The unit and integration tests from this PR are running here https://github.com/tembo-io/pg_vectorize/actions/runs/11631847410

You will also need to add a test for this feature. Keep in mind https://github.com/tembo-io/pg_vectorize/pull/164/files was opened first, so we are reviewing that first.

@akhilender-bongirwar
Copy link
Contributor Author

All of the CI was paused since this PR is from a branch outside the tembo-io org. The unit and integration tests from this PR are running here https://github.com/tembo-io/pg_vectorize/actions/runs/11631847410

You will also need to add a test for this feature. Keep in mind https://github.com/tembo-io/pg_vectorize/pull/164/files was opened first, so we are reviewing that first.

Sure, I understand : )

extension/sql/meta.sql Outdated Show resolved Hide resolved
@akhilender-bongirwar
Copy link
Contributor Author

image

@ChuckHend, please approve workflows once.

extension/sql/meta.sql Outdated Show resolved Hide resolved
@akhilender-bongirwar
Copy link
Contributor Author

@ChuckHend, There are some failing tests, but I didn’t alter them, and they are not related to the changes I made. Please let me know how you would like to proceed.

@ChuckHend
Copy link
Member

Do not worry about test_cohere and test_private_hf_model, since those integration tests will not pass when they are run outside of the tembo org.

I'll have this reviewed within the next couple days! Thank you.

@akhilender-bongirwar
Copy link
Contributor Author

Do not worry about test_cohere and test_private_hf_model, since those integration tests will not pass when they are run outside of the tembo org.

I'll have this reviewed within the next couple days! Thank you.

ok. Thank you ❤️

@ChuckHend
Copy link
Member

Looks great! @varshith257 had a good start on this bounty early and there was clearly value added most recently in this PR from @akhilender-bongirwar. From what I've observed, there were some complementary efforts and I think it would be fair to split the bounty. I will split the bounty as soon as I can figure out how to do that.

@ChuckHend
Copy link
Member

/split @varshith257

@ChuckHend ChuckHend merged commit 4a365d3 into tembo-io:main Dec 13, 2024
3 of 9 checks passed
@akhilender-bongirwar akhilender-bongirwar deleted the feat/event-trigger-for-drop-table branch December 13, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

drop table event triggers for vectorize.jobs
3 participants