-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Destination MSSQL: make Azure Synapse Compatibility In Test #18294
Conversation
@chte thanks for the PR. Can you sign the CLA? |
Hello 👋, first thank you for this amazing contribution. We really appreciate the effort you've made to improve the project. If you have any questions feel free to send me a message in Slack! |
/test connector=connectors/destination-mssql
Build FailedTest summary info:
|
Hello 👋:skin-tone-2: and thank you for your contribution! Airbyte has instituted a code freeze between 19 and 30 December, to make sure there are no disruptions during the holidays. If you have any questions or need further clarification, please don't hesitate to ping via Slack. |
@grishick This is a small change to MSSL. Can it be added to destination team backlog to review and validate it? |
PR to run CI: #23851 |
Integration tests for destionation-mssql are broken and I verified other CI checks on this PR |
Integration tests for destionation-mssql are broken and I verified other CI checks on this PR: airbytehq#23851
@grishick I think this might not work for non-dwh destinations - #24077 (comment) so we might need to make a new connector. Or at least add a config flag for "this is running in azure dwh and needs special handling" base-normalization integration tests are also failing for mssql, with either way - this change hasn't been published yet, so not an urgent thing to solve |
Integration tests for destionation-mssql are broken and I verified other CI checks on this PR: airbytehq#23851
yep. We have to revert this change and make a new connector for Azure Synapse |
What
Describe what the change is solving
Make MSSQL integration test compatible with Azure Synapse
It helps to add screenshots if it affects the frontend.
How
Describe the solution
For Azure DWH a primary key cannot be enforced in Azure DWH. I am only a first day user, I don't entirely know the framework and ideally this should be it's own connector and not lumped together with all MSSQL version.
and this is not tested if non Azure DWH SQL support this constrants
🚨 User Impact 🚨
You can not configure Azure Synapse DWH as a destination
Pre-merge Checklist
I am only a first day user, I don't entirely know the framework and ideally this should be it's own connector and not lumped together with all MSSQL versions. This solution might break other versions of MSSQL and is not tested.
New Connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing/publish
command described hereUpdating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog example