[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #42: METRON-2347: [BRO-PLUGIN-KAFKA] Use docker compose in end to end tests
JonZeolla commented on a change in pull request #42:
URL:
https://github.com/apache/metron-bro-plugin-kafka/pull/42#discussion_r417359367
##
File path: docker/scripts/docker_run_create_topic_in_kafka.sh
##
@@ -80,7 +80,10 @@ echo "Running docker_run_create_topic_in_kafka with "
echo "NETWORK_NAME = $NETWORK_NAME"
echo "==="
-docker run --rm --network "${NETWORK_NAME}" ches/kafka \
+# TODO: Fix this
+sleep 2s
Review comment:
I have a solution for this which is `docker-compose` native, just need
to find some time to wrap it up.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #42: METRON-2347: [BRO-PLUGIN-KAFKA] Use docker compose in end to end tests
JonZeolla commented on a change in pull request #42:
URL:
https://github.com/apache/metron-bro-plugin-kafka/pull/42#discussion_r417050898
##
File path: docker/scripts/docker_run_create_topic_in_kafka.sh
##
@@ -80,7 +80,10 @@ echo "Running docker_run_create_topic_in_kafka with "
echo "NETWORK_NAME = $NETWORK_NAME"
echo "==="
-docker run --rm --network "${NETWORK_NAME}" ches/kafka \
+# TODO: Fix this
+sleep 2s
Review comment:
I may, let me think on it. Was hoping to remove the MIT license and all
the overhead but compose isn't as capable as k8s readiness
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #42: METRON-2347: [BRO-PLUGIN-KAFKA] Use docker compose in end to end tests
JonZeolla commented on a change in pull request #42: URL: https://github.com/apache/metron-bro-plugin-kafka/pull/42#discussion_r417050279 ## File path: docker/containers/kafka/Dockerfile ## @@ -16,20 +14,4 @@ # See the License for the specific language governing permissions and # limitations under the License. # Review comment: Correct, not required. I did it both ways but chose to keep this because I was looking to follow on with moving commands into the containers instead of docker exec/run scripts, maybe making a new entry point. I actually did this work twice because the first time there was too much change and it with have been difficult to review. I can remove for now and add later if you prefer? At the end of the day, it's just the same layers tagged differently This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #42: METRON-2347: [BRO-PLUGIN-KAFKA] Use docker compose in end to end tests
JonZeolla commented on a change in pull request #42: URL: https://github.com/apache/metron-bro-plugin-kafka/pull/42#discussion_r417045900 ## File path: docker/README.md ## @@ -209,33 +140,21 @@ testing scripts to be added to a pull request, and subsequently to a test suite. ```bash --log-directory[REQUIRED] The directory with the logs ``` -- `stop_container.sh`: Stops and removes a Docker container with a given name - ## Parameters - ```bash - --container-name [REQUIRED] The Docker container name - ``` The example end to end test script `run_end_to_end.sh` is provided as an example of a testing script. Specific or extended scripts can be created similar to this script to use the containers. This script does the following: Review comment: Yes because it auto numbers in the rendered version This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #42: METRON-2347: [BRO-PLUGIN-KAFKA] Use docker compose in end to end tests
JonZeolla commented on a change in pull request #42:
URL:
https://github.com/apache/metron-bro-plugin-kafka/pull/42#discussion_r417045301
##
File path: docker/scripts/docker_run_create_topic_in_kafka.sh
##
@@ -80,7 +80,10 @@ echo "Running docker_run_create_topic_in_kafka with "
echo "NETWORK_NAME = $NETWORK_NAME"
echo "==="
-docker run --rm --network "${NETWORK_NAME}" ches/kafka \
+# TODO: Fix this
+sleep 2s
Review comment:
It doesn't work that way. Same with healthcheck. If you look in the
docker compose file I'm already using depends_on
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #42: METRON-2347: [BRO-PLUGIN-KAFKA] Use docker compose in end to end tests
JonZeolla commented on a change in pull request #42:
URL:
https://github.com/apache/metron-bro-plugin-kafka/pull/42#discussion_r417045301
##
File path: docker/scripts/docker_run_create_topic_in_kafka.sh
##
@@ -80,7 +80,10 @@ echo "Running docker_run_create_topic_in_kafka with "
echo "NETWORK_NAME = $NETWORK_NAME"
echo "==="
-docker run --rm --network "${NETWORK_NAME}" ches/kafka \
+# TODO: Fix this
+sleep 2s
Review comment:
It doesn't work that way.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #42: METRON-2347: [BRO-PLUGIN-KAFKA] Use docker compose in end to end tests
JonZeolla commented on a change in pull request #42:
URL:
https://github.com/apache/metron-bro-plugin-kafka/pull/42#discussion_r417045301
##
File path: docker/scripts/docker_run_create_topic_in_kafka.sh
##
@@ -80,7 +80,10 @@ echo "Running docker_run_create_topic_in_kafka with "
echo "NETWORK_NAME = $NETWORK_NAME"
echo "==="
-docker run --rm --network "${NETWORK_NAME}" ches/kafka \
+# TODO: Fix this
+sleep 2s
Review comment:
It doesn't work that way. Same with healthcheck
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #42: METRON-2347: [BRO-PLUGIN-KAFKA] Use docker compose in end to end tests
JonZeolla commented on a change in pull request #42:
URL:
https://github.com/apache/metron-bro-plugin-kafka/pull/42#discussion_r417038403
##
File path: docker/scripts/docker_run_create_topic_in_kafka.sh
##
@@ -80,7 +80,10 @@ echo "Running docker_run_create_topic_in_kafka with "
echo "NETWORK_NAME = $NETWORK_NAME"
echo "==="
-docker run --rm --network "${NETWORK_NAME}" ches/kafka \
+# TODO: Fix this
+sleep 2s
Review comment:
@ottobackwards Open to suggestions - would prefer not need to go back to
using `wait-for-it.sh` but that seems to be a rather [standard
approach](https://docs.docker.com/compose/startup-order/) to solving this
problem.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
