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

Connector builder server: Pin cdk version #21708

Merged
merged 2 commits into from
Jan 23, 2023

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Jan 23, 2023

This is a small adjustment of the dependencies of the connector builder server. Right now the connector builder server is implicitly using the latest version of the CDK. This can lead to broken CI on master as the connector builder server is not automatically rebuilt if the CDK changes, so potential breakages would not be caught in time.

This PR pins the cdk dependency down to the patch version so updating it will touch the connector builder server code and run its tests to make sure the upgrade will actually work.

Another approach would be to automatically test the connector builder server with the current state of cdk as part of CI on the PR itself, but this has several disadvantages:

  • Higher effort to set up in the current system
  • Requires an engineer working on the cdk to fix all problems of the connector builder on the same PR instead of decoupling these streams of work

Just putting this into place for now, but happy to discuss and revisit later on.

@flash1293 flash1293 temporarily deployed to more-secrets January 23, 2023 15:26 — with GitHub Actions Inactive
@flash1293 flash1293 temporarily deployed to more-secrets January 23, 2023 15:26 — with GitHub Actions Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Jan 23, 2023

Airbyte Code Coverage

There is no coverage information present for the Files changed

Total Project Coverage 26.75% 🍏

@flash1293 flash1293 marked this pull request as ready for review January 23, 2023 15:51
@flash1293 flash1293 requested a review from clnoll January 23, 2023 15:51
Copy link
Contributor

@clnoll clnoll left a comment

Choose a reason for hiding this comment

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

LGTM @flash1293 - makes sense that we'd want to do this so we can be more careful as we roll out updates to the connector builder.

@flash1293 flash1293 enabled auto-merge (squash) January 23, 2023 17:50
@flash1293 flash1293 temporarily deployed to more-secrets January 23, 2023 17:52 — with GitHub Actions Inactive
@flash1293 flash1293 temporarily deployed to more-secrets January 23, 2023 17:52 — with GitHub Actions Inactive
@flash1293 flash1293 merged commit e844d73 into master Jan 23, 2023
@flash1293 flash1293 deleted the flash1293/pin-connector-builder-server-cdk-version branch January 23, 2023 18:29
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.

2 participants