-
Notifications
You must be signed in to change notification settings - Fork 13
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
Migrate GitRepositoryAdd to prefect #4788
base: develop-1.1
Are you sure you want to change the base?
Conversation
CodSpeed Performance ReportMerging #4788 will degrade performances by 18.15%Comparing Summary
Benchmarks breakdown
|
75b3430
to
2b969d8
Compare
I would vote to enable the disabled tests related to proposed changes before merging this, i.e. these: @pytest.mark.xfail(reason="FIXME Works locally but it's failling in Github Actions")
async def test_happy_pipeline(self, db: InfrahubDatabase, happy_dataset: None, client: InfrahubClient) -> None:
@pytest.mark.xfail(reason="FIXME Works locally but it's failling in Github Actions")
async def test_conflict_pipeline( Then we need to figure out what's required to actually trigger the jobs to be executed in serial, so that the testing experience is somewhat similar to what we had before. In this PR we see it in the failing integration test where some more work will need to be done, but it could also be other things that are required with regards to these disabled tests. It might be that we need to rewrite them from scratch depending on the options we have with prefect. But if we look at this change: if context.service:
context.service.workflow.submit_workflow(
workflow=GIT_REPOSITORY_ADD, parameters={"model": git_repo_add_model}
) We need the submit_workflow method to actually run the import repo function in the background and add the changes back to the current test instance in the same way as the old RabbitMQ message did. |
That should already work as expected when the tests are using |
Ah, right so will just need to update that integration test. I still think it would be good to have the other tests enabled again though. |
No description provided.