-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
🎉 Source Harvest: add OAuth 2.0 #7952
Conversation
Signed-off-by: Sergey Chvalyuk <[email protected]>
Signed-off-by: Sergey Chvalyuk <[email protected]>
Signed-off-by: Sergey Chvalyuk <[email protected]>
Signed-off-by: Sergey Chvalyuk <[email protected]>
/test connector=connectors/source-harvest
|
Signed-off-by: Sergey Chvalyuk <[email protected]>
/test connector=connectors/source-harvest
|
/test connector=connectors/source-harvest
|
Signed-off-by: Sergey Chvalyuk <[email protected]>
/test connector=connectors/source-harvest
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add basic tests for Java part. You can use examples of other ready connectors.
airbyte-integrations/connectors/source-harvest/source_harvest/auth.py
Outdated
Show resolved
Hide resolved
/test connector=connectors/source-harvest
|
"complete_oauth_output_specification": { | ||
"refresh_token": { | ||
"type": "string", | ||
"path_in_connector_config": ["credentials", 0, "refresh_token"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"path_in_connector_config": ["credentials", 0, "refresh_token"] | |
"path_in_connector_config": ["credentials", "refresh_token"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
"complete_oauth_server_output_specification": { | ||
"client_id": { | ||
"type": "string", | ||
"path_in_connector_config": ["credentials", 0, "client_id"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"path_in_connector_config": ["credentials", 0, "client_id"] | |
"path_in_connector_config": ["credentials", "client_id"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
}, | ||
"client_secret": { | ||
"type": "string", | ||
"path_in_connector_config": ["credentials", 0, "client_secret"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"path_in_connector_config": ["credentials", 0, "client_secret"] | |
"path_in_connector_config": ["credentials", "client_secret"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
"oauth_config_specification": { | ||
"complete_oauth_output_specification": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"oauth_config_specification": { | |
"complete_oauth_output_specification": { | |
"oauth_config_specification": { | |
"predicate_key": ["credentials", "auth_type"], | |
"predicate_value": "Client", | |
"complete_oauth_output_specification": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it mistake here ?
I see that predicate_key
not inside oauth_config_specification
airbyte/airbyte-api/src/main/openapi/config.yaml
Line 3255 in c5a7267
predicateKey: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh you're right then it's:
"oauth_config_specification": { | |
"complete_oauth_output_specification": { | |
"predicate_key": ["credentials", "auth_type"], | |
"predicate_value": "Client", | |
"oauth_config_specification": { | |
"complete_oauth_output_specification": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The object inside:
- complete_oauth_output_specification
- complete_oauth_server_input_specification
- complete_oauth_server_output_specification
needs to be JSON schema object, so you need to add a layer of
"type": "object",
"additionalProperties": false,
"properties": {
"complete_oauth_server_input_specification": { | ||
"client_id": { | ||
"type": "string" | ||
}, | ||
"client_secret": { | ||
"type": "string" | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"complete_oauth_server_input_specification": { | |
"client_id": { | |
"type": "string" | |
}, | |
"client_secret": { | |
"type": "string" | |
} | |
}, | |
"complete_oauth_server_input_specification": { | |
"type": "object", | |
"additionalProperties": "false", | |
"properties": { | |
"client_id": { | |
"type": "string" | |
}, | |
"client_secret": { | |
"type": "string" | |
} | |
} | |
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Signed-off-by: Sergey Chvalyuk <[email protected]>
/test connector=connectors/source-harvest
|
@@ -224,7 +224,6 @@ read_secrets source-google-sheets "$GOOGLE_SHEETS_TESTS_CREDS_OLD" "old_config.j | |||
read_secrets source-google-workspace-admin-reports "$GOOGLE_WORKSPACE_ADMIN_REPORTS_TEST_CREDS" | |||
read_secrets source-greenhouse "$GREENHOUSE_TEST_CREDS" | |||
read_secrets source-greenhouse "$GREENHOUSE_TEST_CREDS_LIMITED" "config_users_only.json" | |||
read_secrets source-harvest "$HARVEST_INTEGRATION_TESTS_CREDS" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious why are these removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are switching from Github secrets to Google Secrets. New approach based on google secrets allow to dynamically generate secrets/config.json for current connector without this hard-coded variable.
Signed-off-by: Sergey Chvalyuk <[email protected]>
/publish connector=connectors/source-harvest
|
Signed-off-by: Sergey Chvalyuk <[email protected]>
* add OAuth 2.0 for source-harvest Signed-off-by: Sergey Chvalyuk <[email protected]>
Signed-off-by: Sergey Chvalyuk [email protected]
How
Add an implementation of oAuth2 logic to the connector and the Java server part
Manual result of tests:
2. Auth result:
Pre-merge Checklist
Expand the relevant checklist and delete the others.
Updating 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 exampleAirbyter
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 here