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.

Sign up for GitHub

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

Jump to bottom

Surveymonkey: oAuth backend. #7524

Merged
merged 3 commits into from
Nov 2, 2021
Merged

Surveymonkey: oAuth backend. #7524

merged 3 commits into from
Nov 2, 2021

Conversation

avida
Copy link
Contributor

@avida avida commented Nov 1, 2021

Resovles #7510

@github-actions github-actions bot added area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Nov 1, 2021
@avida avida temporarily deployed to more-secrets November 1, 2021 13:29 Inactive
sherifnada
sherifnada approved these changes Nov 1, 2021
View reviewed changes
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

java LGTM, though I suggest we improve the python connctor spec like described in the comment as part of this PR

airbyte-integrations/connectors/source-surveymonkey/source_surveymonkey/spec.json Outdated
@@ -17,16 +17,12 @@
"credentials": {
"type": "object",
"title": "Authentication Type",
"description": "Authenticate over oAuth flow or provide token",
"oneOf": [
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can even go a step further and put the entire auth block in the root object. There's no reason really to have two auth mechanisms in a oneOf, since the connector always takes the access token. This should probably look like the FB marketing oauth spec.

FWIW I don't think we should have merged this connector in this current state, see: #7433 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

airbyte-oauth/src/test/java/io/airbyte/oauth/flows/SurveymonkeyOAuthFlowTest.java
}

@Test
public void testCompleteDestinationOAuth() throws IOException, ConfigNotFoundException, InterruptedException {
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we testing destination? sourcemonkey is only a source ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trying to test all methods from OAuthFlowImplementation interface.

@avida avida linked an issue Nov 1, 2021 that may be closed by this pull request
oAuth backend for survey-monkey source #7510
Closed
@avida avida temporarily deployed to more-secrets November 2, 2021 08:21 Inactive
@avida avida mentioned this pull request Nov 2, 2021
Closed
@avida avida force-pushed the drezchykov/oauth-surveymonkey branch from c565498 to 99878af Compare November 2, 2021 08:26
@avida
Copy link
Contributor Author

avida commented Nov 2, 2021 •
edited by github-actions bot
Loading

/publish connector=connectors/source-surveymonkey

🕑 connectors/source-surveymonkey https://github.com/airbytehq/airbyte/actions/runs/1411413401
✅ connectors/source-surveymonkey https://github.com/airbytehq/airbyte/actions/runs/1411413401

@avida avida temporarily deployed to more-secrets November 2, 2021 08:29 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets November 2, 2021 08:31 Inactive
@avida avida merged commit 85381bd into master Nov 2, 2021
@avida avida deleted the drezchykov/oauth-surveymonkey branch November 2, 2021 08:40
@jrhizor jrhizor mentioned this pull request Nov 3, 2021
Merged
lmossman pushed a commit that referenced this pull request Nov 3, 2021
@lmossman
Surveymonkey: oAuth backend and connector's spec change. (#7524)
6168f28
schlattk pushed a commit to schlattk/airbyte that referenced this pull request Jan 4, 2022
@schlattk
Surveymonkey: oAuth backend and connector's spec change. (airbytehq#7524
e3d3187 
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sherifnada sherifnada sherifnada approved these changes

@ChristopheDuong ChristopheDuong Awaiting requested review from ChristopheDuong

@eliziario eliziario Awaiting requested review from eliziario

Assignees
No one assigned
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

oAuth backend for survey-monkey source
3 participants
@avida @sherifnada @jrhizor