-
Notifications
You must be signed in to change notification settings - Fork 231
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
DBZ-4128 Add Debezium Kafka Connect REST Extension to Debezium Kafka Connect NIGHTLY container image #259
Conversation
…Connect NIGHTLY container image * minor cleanup closes https://issues.redhat.com/browse/DBZ-4128
RUN for PACKAGE in {scripting}; do \ | ||
SNAPSHOT_VERSION=$(curl -fSL $MAVEN_OSS_SNAPSHOT/io/debezium/debezium-$PACKAGE/$DEBEZIUM_VERSION/maven-metadata.xml | awk -F'<[^>]+>' '/<extension>tar.gz<\/extension>/ {getline; print $2}'); \ | ||
curl -fSL -o /tmp/package.tar.gz \ | ||
RUN for PACKAGE in {scripting,}; do \ |
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.
Any particular reason for the loop given it's a single element? Same below?
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.
yes, it was like that before. I guess good old premature optimization. that's the correct Bash syntax for a list and I only added the missing comma, else Bash would handle it as the string {scripting}
and not as list.
curl -fSL -o /tmp/plugin.tar.gz \ | ||
$MAVEN_OSS_SNAPSHOT/io/debezium/debezium-connector-$CONNECTOR/$DEBEZIUM_VERSION/debezium-connector-$CONNECTOR-$SNAPSHOT_VERSION-plugin.tar.gz &&\ | ||
SNAPSHOT_VERSION=$(curl --silent -fSL $MAVEN_OSS_SNAPSHOT/io/debezium/debezium-connector-$CONNECTOR/$DEBEZIUM_VERSION/maven-metadata.xml | awk -F'<[^>]+>' '/<extension>tar.gz<\/extension>/ {getline; print $2; exit}'); \ | ||
echo "Downloading and installing debezium-connector-$CONNECTOR-$SNAPSHOT_VERSION-plugin.tar.gz ..." ; \ |
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.
Why the semicolons are used as command delimiter here and not &&
as below?
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.
Because &&
is a logical connection. But the next command can be executed when the echo doesn't work. And in general echo should always return true if a tty is attached. The other &&
are used to continue with following commands only when the previous succeeded.
done | ||
done | ||
|
||
RUN for PACKAGE in {connect-rest-extension,}; do \ |
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.
Would it be possible to merge the two extension
loops? IIUC they only differ in the filename suffix.
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.
No, they also differ in what they evaluate from the maven-metadata.xml
.
@@ -11,17 +11,32 @@ ENV DEBEZIUM_VERSION=$DEBEZIUM_VERSION \ | |||
# Download the snapshot version of the connectors and then install them into the `$KAFKA_CONNECT_PLUGINS_DIR/debezium` directory... | |||
# | |||
RUN for CONNECTOR in {mysql,mongodb,postgres,sqlserver,oracle,db2,vitess}; do \ |
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.
As you are touching the file, would it be possible to intorduce two env vars on top of the file, one enumerating plugins and the other extensions and refer to them in the loop?
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 could do it as an ARG
so that we can build a different connector list based on a build arg. But do we really need this anytime soon?
I expected this to be already merged as I need the container image to finish the DBZ UI tests and next week Thursday is my last day before my PTO. |
closes https://issues.redhat.com/browse/DBZ-4128