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.

Sign up for GitHub

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

Jump to bottom

Update config and job persistence unit tests to run migrations #5857

Closed
tuliren opened this issue Sep 6, 2021 · 1 comment · Fixed by #7219
Closed

Update config and job persistence unit tests to run migrations #5857

tuliren opened this issue Sep 6, 2021 · 1 comment · Fixed by #7219
Assignees
@tuliren
Labels
type/bug Something isn't working

Comments

@tuliren
Copy link
Contributor

tuliren commented Sep 6, 2021

Current Behavior

After introducing Flyway migration, any unit test that depends on the database need to run the migrations first. Currently most such unit tests do not do this. It is not a problem now because there is no Flyway migration yet. But we will run into this problem soon.

Expected Behavior

Unit test should work after Flyway migrations are introduced in the future.

Are you willing to submit a PR?

Yes.
@tuliren tuliren added the type/bug Something isn't working label Sep 6, 2021
@tuliren tuliren self-assigned this Sep 6, 2021
@tuliren tuliren mentioned this issue Sep 6, 2021
Merged
38 tasks
davinchia added a commit that referenced this issue Sep 9, 2021
@davinchia
🐛 Fix cancel not working on Kube deployment since 0.29.13-alpha. (#5850)
7aa4c71 
A few days ago we removed the workflow volume from the Kubernetes deployment in order to simplify the set up. This also lets us do away with the workspace PVC.

In theory this isn't needed since the volume is mainly used for logs and our Kube deployment logs out to the Cloud storage.

In the process of doing so, we realised the volume is used to store the temporal workflow id that is later used to cancel the workflow. Thus cancellations stopped working.

This PR:

Adds a migration to add the temporalWorkflowId column to the Attempts table. Exposes various persistent methods for this.
Modify temporal to store the workflow id in this column. Modify cancellation to retrieve the workflow id from the table.
Things to call out:

This approach means the worker now requires access to the jobs DB. I think this is reasonable.
Some tests are disabled since we haven't really stabilised the Flyway + older file-base migrations yet. Follow up ticket has been created (Update config and job persistence unit tests to run migrations #5857) and Liren is working on this.
@tuliren tuliren mentioned this issue Oct 26, 2021
Merged
@tuliren
Copy link
Contributor Author

tuliren commented Oct 26, 2021

This issue has been resolved in #7219. Use TestDatabaseProviders to create databases in unit test. It will take care of the initialization and migration.

@tuliren tuliren closed this as completed Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tuliren tuliren

Labels
type/bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Copy job attempt state to configs database airbytehq/airbyte
1 participant
@tuliren