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

🎉 New Source: KYVE #26299

Closed
wants to merge 15 commits into from
Closed

🎉 New Source: KYVE #26299

wants to merge 15 commits into from

Conversation

cgkournelos-rf
Copy link
Contributor

@cgkournelos-rf cgkournelos-rf commented May 19, 2023 •
edited
Loading

What

New source connector for KYVE Network. Related issue #26222.

How

Developed using (Configuration Based Source) low-code CDK

Tests

Acceptance Test Results :
image

🚨 User Impact 🚨

No breaking changes

Pre-merge Actions

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

  • PR name follows PR naming conventions
  • Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan and you've followed all steps in the Breaking Changes Checklist
  • Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
  • Secrets in the connector's spec are annotated with airbyte_secret
  • All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
  • Changelog updated in docs/integrations//.md with an entry for the new version. See changelog example
  • You, or an Airbyter, have run /test successfully on this PR - or on a non-forked branch
  • You, or an Airbyter, have run /publish successfully on this PR - or on a non-forked branch
  • You've updated the connector's metadata.yaml file new!

@CLAassistant
Copy link

CLAassistant commented May 19, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation community connectors/source/kyve labels May 19, 2023
@github-actions
Copy link
Contributor

Before Merging a Connector Pull Request

Wow! 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:

  • PR name follows PR naming conventions
  • Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan and you've followed all steps in the Breaking Changes Checklist
  • Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
  • Secrets in the connector's spec are annotated with airbyte_secret
  • All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
  • Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
  • You, or an Airbyter, have run /test successfully on this PR - or on a non-forked branch
  • You, or an Airbyter, have run /publish successfully on this PR - or on a non-forked branch
  • You've updated the connector's metadata.yaml file new!

If the checklist is complete, but the CI check is failing,

  1. Check for hidden checklists in your PR description

  2. Toggle the github label checklist-action-run on/off to re-run the checklist CI.

@marcosmarxm
Copy link
Member

@cgkournelos-rf make sure all contributors sign the CLA.

@cgkournelos-rf cgkournelos-rf marked this pull request as ready for review May 24, 2023 20:11
@cgkournelos-rf cgkournelos-rf requested a review from a team as a code owner May 24, 2023 21:43
@octavia-squidington-iii octavia-squidington-iii added the CDK Connector Development Kit label May 24, 2023
@cgkournelos-rf cgkournelos-rf marked this pull request as draft May 24, 2023 22:45
@octavia-squidington-iii octavia-squidington-iii removed the CDK Connector Development Kit label May 25, 2023
@marcosmarxm
Copy link
Member

@cgkournelos-rf make sure all contributors sign the CLA.

@cgkournelos-rf make sure you check this comment.

@cgkournelos-rf cgkournelos-rf marked this pull request as ready for review June 1, 2023 09:11
@cgkournelos-rf cgkournelos-rf requested a review from sherifnada June 1, 2023 10:07
@cgkournelos-rf cgkournelos-rf marked this pull request as draft June 2, 2023 08:48
@cgkournelos-rf cgkournelos-rf marked this pull request as ready for review June 2, 2023 09:06
@cgkournelos-rf
Copy link
Contributor Author

@marcosmarxm This PR is ready for review and from our side, we have completed the checklist.

We are not sure how to trigger the CI test for completing the checklist.

@sajarin sajarin removed the request for review from sherifnada June 2, 2023 15:28
@sajarin sajarin self-assigned this Jun 2, 2023
@cgkournelos-rf cgkournelos-rf requested a review from sajarin June 12, 2023 09:07
@cgkournelos-rf
Copy link
Contributor Author

@sajarin We have addressed your comments!

Let us know in case anything else is missing.

@kostas-christ3
fix: Address review issues
57f39e9 
- Fixed Dockefile and changelog version to 0.1.0
- Added order property on spec.yaml
- AdditionalProperties to the proper objects in schema definitions and made all fields nullable
@sajarin
Copy link
Contributor

sajarin commented Jun 12, 2023 •
edited
Loading

@cgkournelos-rf thanks, this looks good. Would you by chance be able to provide credentials to test the connector on our side? It's not required but would be beneficial for getting this PR merged.

Edit: Otherwise, we will need to disable the integration tests in the acceptance-test-config.yaml file

@cgkournelos-rf
Copy link
Contributor Author

@sajarin thanks for the feedback. Sure it will be great if you want to test it on your side. However, we are not sure what kind of credentials you require.

sajarin
sajarin reviewed Jun 14, 2023
View reviewed changes
airbyte-config-oss/init-oss/src/main/resources/icons/kyve.svg
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this svg file to the connector folder, we no longer store icons in the airbyte-config-oss folder. More info here: https://docs.airbyte.com/connector-development/connector-metadata-file/#the-icon-field

sajarin
sajarin suggested changes Jun 14, 2023
View reviewed changes
Copy link
Contributor

@sajarin sajarin left a comment

Choose a reason for hiding this comment

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

Just an additional minor comment

@marcosmarxm marcosmarxm mentioned this pull request Jun 14, 2023
Merged
@marcosmarxm
Copy link
Member

Merged at #27373

@cgkournelos-rf cgkournelos-rf deleted the source-kyve branch August 7, 2023 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cgkournelos cgkournelos cgkournelos left review comments

@sherifnada sherifnada sherifnada left review comments

@sajarin sajarin sajarin requested changes

Assignees

@sajarin sajarin

Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation community connectors/source/kyve
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@cgkournelos-rf @CLAassistant @marcosmarxm @sajarin @sherifnada @cgkournelos @octavia-squidington-iii @fabianriewe @kostas-christ3