Introduce webhook configs into workspace api and persistence #17950
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
area/api
mfsiega-airbyte
changed the title
davinchia
reviewed
Oct 13, 2022
davinchia
reviewed
Oct 13, 2022
davinchia
reviewed
Oct 13, 2022
davinchia
reviewed
Oct 13, 2022
davinchia
reviewed
Oct 13, 2022
airbyte-config/config-models/src/main/resources/types/WebhookOperationConfigs.yaml
Show resolved
Hide resolved
davinchia
reviewed
Oct 13, 2022
airbyte-config/config-models/src/main/resources/types/StandardWorkspace.yaml
Show resolved
Hide resolved
davinchia
reviewed
Oct 13, 2022
| @@ -766,6 +766,8 @@ private void writeStandardWorkspace(final List<StandardWorkspace> configs, final | |||
| .set(WORKSPACE.FIRST_SYNC_COMPLETE, standardWorkspace.getFirstCompletedSync()) | |||
| .set(WORKSPACE.FEEDBACK_COMPLETE, standardWorkspace.getFeedbackDone()) | |||
| .set(WORKSPACE.UPDATED_AT, timestamp) | |||
| .set(WORKSPACE.WEBHOOK_OPERATION_CONFIGS, standardWorkspace.getWebhookOperationConfigs() == null ? null | |||
There was a problem hiding this comment.
note: we should make sure this is tested
There was a problem hiding this comment.
Planning on this being an e2e test (in a subsequent PR).
davinchia
reviewed
Oct 13, 2022
airbyte-server/src/main/java/io/airbyte/server/handlers/WorkspacesHandler.java
Outdated
Show resolved
Hide resolved
davinchia
reviewed
Oct 13, 2022
airbyte-server/src/main/java/io/airbyte/server/converters/WebhookOperationConfigsConverter.java
Outdated
Show resolved
Hide resolved
mfsiega-airbyte
commented
Oct 13, 2022
.../config-persistence/src/main/java/io/airbyte/config/persistence/SecretsRepositoryWriter.java
Outdated
Show resolved
Hide resolved
davinchia
reviewed
Oct 13, 2022
| .withWebhookOperationConfigs(Jsons.jsonNode(webhookConfigs)); | ||
| secretsRepositoryWriter.writeWorkspace(workspace); | ||
| final var workspaceArgumentCaptor = ArgumentCaptor.forClass(StandardWorkspace.class); | ||
| verify(configRepository, times(1)).writeStandardWorkspaceNoSecrets(workspaceArgumentCaptor.capture()); |
davinchia
reviewed
Oct 13, 2022
| @@ -433,4 +444,37 @@ void testWriteDifferentStagingConfiguration() throws JsonValidationException, Co | |||
| Map.of(PASSWORD_FIELD_NAME, hmacSecretNewCoordinate.getFullCoordinate())))); | |||
| } | |||
|
|
|||
| @Test | |||
| void testWriteWorkspaceSplitsSecrets() throws JsonValidationException, IOException { | |||
There was a problem hiding this comment.
Suggested change
| void testWriteWorkspaceSplitsSecrets() throws JsonValidationException, IOException { | |
| void testWriteWorkspaceSplitsAuthToken() throws JsonValidationException, IOException { |
davinchia
reviewed
Oct 13, 2022
| @@ -433,4 +444,37 @@ void testWriteDifferentStagingConfiguration() throws JsonValidationException, Co | |||
| Map.of(PASSWORD_FIELD_NAME, hmacSecretNewCoordinate.getFullCoordinate())))); | |||
| } | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
can also use the @DisplayName annotation to clarify this
davinchia
reviewed
Oct 13, 2022
| @@ -461,4 +466,42 @@ void testSetFeedbackDone() throws JsonValidationException, ConfigNotFoundExcepti | |||
| verify(configRepository).setFeedback(workspaceGiveFeedback.getWorkspaceId()); | |||
| } | |||
|
|
|||
| @Test | |||
| void testWorkspaceIsWrittenThroughSecretsWriter() throws JsonValidationException, ConfigNotFoundException, IOException { | |||
There was a problem hiding this comment.
the ConfigNotFoundException is not used
davinchia
reviewed
Oct 13, 2022
airbyte-server/src/test/java/io/airbyte/server/handlers/WorkspacesHandlerTest.java
Show resolved
Hide resolved
davinchia
approved these changes
Oct 13, 2022
letiescanciano
added a commit
that referenced
this pull request
Oct 14, 2022
…vation * master: (98 commits) 🐛 Source Bing Ads - Fix Campaigns stream misses Audience and Shopping (#17873) Source S3 - fix schema inference (#17991) 🎉 JDBC sources: store cursor record count in db state (#15535) Introduce webhook configs into workspace api and persistence (#17950) ci: upload test results to github for analysis (#17953) Trigger the connectors build if there are worker changes. (#17976) Add additional sync timing information (#17643) Use page_token_option instead of page_token (#17892) capture metrics around json messages size (#17973) 🐛 Correct kube annotations variable as per the docs. (#17972) 🪟 🎉 Add /connector-builder page with embedded YAML editor (#17482) fix `est_num_metrics_emitted_by_reporter` not being emitted (#17929) Update schema dumps (#17960) Remove the bump in the value.yml (#17959) Ensure database initialization in test container (#17697) Remove typo line from incremental reads docs (#17920) DocS: Update authentication.md (#17931) Use MessageMigration for Source Connection Check. (#17656) fixed links (#17949) remove usages of YamlSeedConfigPersistence (#17895) ...
jhammarstedt
pushed a commit
to jhammarstedt/airbyte
that referenced
this pull request
Oct 31, 2022
…hq#17950) * wip * handle webhook configs in workspaces endpoint and split/hydrate secrets * style improvements to documentation around webhook configs * Clarify documentation around webhook auth tokens * More documentation clarification around webhook configs * Format. * unit test coverage for webhook config handling * use common json parsing libraries around webhook configs * clean up around testing webhook operation configs Co-authored-by: Davin Chia <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Introduce webhook configs into the workspace API and persistence layers.
How
Recommended reading order
airbyte-api/src/main/openapi/config.yamlairbyte-config/config-models/src/main/resources/types/WebhookOperationConfigs.yamlairbyte-config/config-models/src/main/resources/types/StandardWorkspace.yamlairbyte-server/src/main/java/io/airbyte/server/handlers/WorkspacesHandler.javaTesting
This has been manually tested locally. Adding automated tests is left as a TODO. The existing tests -- which do not exercise the webhook config handling -- should still pass.