-
Notifications
You must be signed in to change notification settings - Fork 4
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
Adding s3 bucket sink #5
Conversation
@matzew: The label(s) In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
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 sink-connector has mixed a few things coming from a source-connector (e.g. in kn-connector-source.yaml which should rather follow the concepts of kn-connector-sink.yaml).
Please have a look into log-sink connector as an example or use the Maven archetype for sink connectors to generate the project
mvn archetype:generate \
-DarchetypeGroupId=dev.knative.eventing \
-DarchetypeArtifactId=kn-connector-archetype-sink \
-DarchetypeVersion=1.0-SNAPSHOT
aws-s3-sink/pom.xml
Outdated
<scope>test</scope> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.eclipse.jetty.ee10</groupId> |
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 dependency is not required anymore as we have pinned the version in dependencyManagement in parent POM
# Knative Camel component | ||
camel.component.knative.environmentPath=classpath:knative.json | ||
|
||
camel.component.knative.ceOverride[ce-type]={{kn.connector.ce.override.type:dev.knative.connector.event.aws-s3}} |
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.
these ceOverrides are not required for the sink
metadata: | ||
name: kn-connector-aws-s3-sink | ||
spec: | ||
source: |
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.
source should be the Knative Broker
apiVersion: camel.apache.org/v1 | ||
kind: Kamelet | ||
name: aws-s3-sink | ||
sink: |
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.
sink should be the aws-s3-sink Kamelet
Signed-off-by: Matthias Wessendorf <[email protected]>
I used the archetype now as:
the I reverted the change from the archetype since it was adding new lines after each existing line. Was very verbose |
yes, Maven archetype process goes wild on formatting the existing POM. Revert and adding the module manually is the best option for now. thanks |
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.
LGTM now
@@ -0,0 +1,4 @@ | |||
|=== | |||
|Property |Required |EnvVar |Description |
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.
consider running mvn package
once before the PR, so the docs get generated
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: christophd, matzew The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
dce829d
into
knative-extensions:main
Changes
/kind
Fixes #
Release Note
Docs