-
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
🎉 New Source: Serpstat #28147
🎉 New Source: Serpstat #28147
Conversation
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
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.
I left a few comments, sorry the delay to review the contribution.
airbyte-integrations/connectors/source-serpstat/source_serpstat/schemas/TODO.md
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-serpstat/source_serpstat/schemas/customers.json
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-serpstat/source_serpstat/schemas/employees.json
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-serpstat/source_serpstat/spec.yaml
Show resolved
Hide resolved
airbyte-integrations/connectors/source-serpstat/source_serpstat/spec.yaml
Show resolved
Hide resolved
airbyte-integrations/connectors/source-serpstat/source_serpstat/manifest.yaml
Outdated
Show resolved
Hide resolved
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.
@alex-danilin I have a sandbox account and the connector is working. One comment and it's almost done!! Thanks for the contribution.
airbyte-integrations/connectors/source-serpstat/source_serpstat/manifest.yaml
Outdated
Show resolved
Hide resolved
@alex-danilin when I'm testing the connector I keep getting errors. Can you investigate that? Particularly for Domain History. {
"status": 200,
"body": {
"id": "2023-08-04 13:50:27.856314+00:00",
"error": {
"code": 32000,
"message": "Too Many Requests",
"data": null
}
}, Updating the Stop condition to: |
@marcosmarxm I've added "Too many requests" handling to my latest commit as you proposed. Can you please check and if the issue still exists please send me the connector setting that you are using for me to reproduce it exactly as you have. |
@alex-danilin can you allow me to push code changes to your branch? There are a few updates need to be done in your code. |
@marcosmarxm I've sent you the invite to my branch and fixed all the points you mentioned. Check, please. |
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.
Congrats @alex-danilin
@marcosmarxm Thanks for your assistance. |
Co-authored-by: Marcos Marx <[email protected]> Co-authored-by: marcosmarxm <[email protected]>
What
The initial release of Serpstat source connector related to #28082
How
The initial release of Serpstat source connector
Pre-merge Actions
Community member or Airbyter
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.0.0.1
Dockerfile
has version0.0.1
README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog with an entry for the initial version. See changelog exampledocs/integrations/README.md
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.