Skip to content

🐛 octavia-cli: propagate open api spec update #11441

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

Merged
merged 11 commits into from
Mar 29, 2022

Conversation

alafanechere
Copy link
Contributor

@alafanechere alafanechere commented Mar 25, 2022

What

Closes #11438

How

  • Write integration tests to spot regressions introduced by API changes:
    • Reproduce management of source, destination and connection in octavia-cli/integration_tests/test_apply/test_resources.py . The test configuration files explain the number of new lines in this PR 😄
  • Update the model we use to retrieve the source catalog from the API to propagate changes made to the Open API spec in 3d9f9ec . Change from SourceIdRequestBody to SourceDiscoverSchemaRequestBody .
  • Run integration tests from the CI (update octavia build job and run a script to run the integration test on a new airbyte instance).

Todo

  • Write the same kind of integration test for connection management.
  • Run the integration tests in the CI to make builds fail if the test does not pass. The octavia build workflow is triggered if changes are detected in airbyte-api folder (and on the octavia-cli folder of course).

@alafanechere alafanechere marked this pull request as ready for review March 25, 2022 18:14
@alafanechere alafanechere added this to the octavia-cli-alpha milestone Mar 25, 2022
@alafanechere alafanechere temporarily deployed to more-secrets March 28, 2022 07:01 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 28, 2022 07:02 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 28, 2022 07:14 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 28, 2022 07:14 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 28, 2022 07:54 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 28, 2022 07:55 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 28, 2022 08:13 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 28, 2022 08:13 Inactive
@alafanechere alafanechere requested a review from davinchia March 28, 2022 10:27
@alafanechere alafanechere temporarily deployed to more-secrets March 28, 2022 10:29 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 28, 2022 10:29 Inactive
Copy link
Contributor

@lmossman lmossman left a comment

Choose a reason for hiding this comment

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

Had a couple questions about the tests but I don't think they are blocking. The other changes here seem fine to me!

@pytest.fixture(scope="module")
def source_state_path():
state_path = f"{os.path.dirname(__file__)}/configurations/sources/poke/state.yaml"
silent_remove(state_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really understand why these methods are calling remove on state_path? Or why its calling silent_remove() twice?

And similarly, why are the source() and destination() methods calling delete_source()/delete_destination() after yielding them?

Copy link
Contributor Author

@alafanechere alafanechere Mar 29, 2022

Choose a reason for hiding this comment

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

In the context of pytest fixtures, the code after yielding is considered as the test teardown logic. I'm basically cleaning up state files and remote resources at the end of the test run.

@alafanechere alafanechere merged commit 55ae3f8 into master Mar 29, 2022
@alafanechere alafanechere deleted the augustin/octavia-cli/propagate-api-update branch March 29, 2022 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 octavia-cli: generate connection fails
2 participants