You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "shreemaan-abhishek (via GitHub)" <gi...@apache.org> on 2023/05/08 21:09:53 UTC
[GitHub] [apisix] shreemaan-abhishek opened a new pull request, #9437: chore(ci): use real clickhouse server in ci
shreemaan-abhishek opened a new pull request, #9437:
URL: https://github.com/apache/apisix/pull/9437
### Description
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
Fixes # (issue)
### Checklist
- [ ] I have explained the need for this PR and the problem it solves
- [ ] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [ ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shreemaan-abhishek commented on a diff in pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on code in PR #9437:
URL: https://github.com/apache/apisix/pull/9437#discussion_r1188065235
##########
ci/pod/docker-compose.plugin.yml:
##########
@@ -324,10 +324,21 @@ services:
networks:
vector_net:
+ clickhouse:
+ image: clickhouse/clickhouse-server:23.4.2-alpine
+ container_name: clickhouse
+ ports:
+ - '8123:8123'
+ - '9000:9000'
Review Comment:
Just in case we need those ports in the future, we will not have to redeclare it again. That's why I bound all the ports available.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Sn0rt commented on pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "Sn0rt (via GitHub)" <gi...@apache.org>.
Sn0rt commented on PR #9437:
URL: https://github.com/apache/apisix/pull/9437#issuecomment-1554282074
> It is better to change the old test cases and use the real clickhouse sever
@shreemaan-abhishek
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shreemaan-abhishek commented on pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on PR #9437:
URL: https://github.com/apache/apisix/pull/9437#issuecomment-1539059691
The reviewers might request to use the real clickhouse server everywhere in the test suite and eliminate the mock server. We can do that but if we remove the mock server we need to create another clickhouse server to test the multiple endpoints feature.
Incrementally adding docker images/containers to ci will make it slower which is not good in the long run. WDYT? 🤔
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Sn0rt commented on a diff in pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "Sn0rt (via GitHub)" <gi...@apache.org>.
Sn0rt commented on code in PR #9437:
URL: https://github.com/apache/apisix/pull/9437#discussion_r1188027184
##########
.github/workflows/build.yml:
##########
@@ -136,6 +136,7 @@ jobs:
make ci-env-up project_compose_ci=ci/pod/docker-compose.${{ steps.test_env.outputs.type }}.yml
[[ ${{ steps.test_env.outputs.type }} != first ]] && sudo ./ci/init-${{ steps.test_env.outputs.type }}-test-service.sh after
echo "Linux launch services, done."
+
Review Comment:
remove this newline (it's not needed
##########
ci/pod/docker-compose.plugin.yml:
##########
@@ -324,10 +324,21 @@ services:
networks:
vector_net:
+ clickhouse:
+ image: clickhouse/clickhouse-server:23.4.2-alpine
+ container_name: clickhouse
+ ports:
+ - '8123:8123'
+ - '9000:9000'
Review Comment:
why create 9000? I only find the port 8123 port has been used.
##########
ci/init-plugin-test-service.sh:
##########
@@ -51,6 +51,8 @@ after() {
# configure keycloak
docker exec apisix_keycloak bash /tmp/kcadm_configure_cas.sh
docker exec apisix_keycloak bash /tmp/kcadm_configure_university.sh
+
+ echo "CREATE TABLE default.test (\`host\` String, \`client_ip\` String, \`route_id\` String, \`service_id\` String, \`@timestamp\` String, PRIMARY KEY(\`@timestamp\`)) ENGINE = MergeTree()" | curl 'http://localhost:8123/'
Review Comment:
should we set the logger format? or is this the default struct?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shreemaan-abhishek commented on a diff in pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on code in PR #9437:
URL: https://github.com/apache/apisix/pull/9437#discussion_r1188069307
##########
ci/init-plugin-test-service.sh:
##########
@@ -51,6 +51,8 @@ after() {
# configure keycloak
docker exec apisix_keycloak bash /tmp/kcadm_configure_cas.sh
docker exec apisix_keycloak bash /tmp/kcadm_configure_university.sh
+
+ echo "CREATE TABLE default.test (\`host\` String, \`client_ip\` String, \`route_id\` String, \`service_id\` String, \`@timestamp\` String, PRIMARY KEY(\`@timestamp\`)) ENGINE = MergeTree()" | curl 'http://localhost:8123/'
Review Comment:
This was given in the documentation, of course we can use a different format as well.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Revolyssup commented on pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "Revolyssup (via GitHub)" <gi...@apache.org>.
Revolyssup commented on PR #9437:
URL: https://github.com/apache/apisix/pull/9437#issuecomment-1554425062
@Sn0rt @shreemaan-abhishek Any reason why we are not using vector here?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Sn0rt commented on a diff in pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "Sn0rt (via GitHub)" <gi...@apache.org>.
Sn0rt commented on code in PR #9437:
URL: https://github.com/apache/apisix/pull/9437#discussion_r1188027892
##########
ci/pod/docker-compose.plugin.yml:
##########
@@ -324,10 +324,21 @@ services:
networks:
vector_net:
+ clickhouse:
+ image: clickhouse/clickhouse-server:23.4.2-alpine
+ container_name: clickhouse
+ ports:
+ - '8123:8123'
+ - '9000:9000'
Review Comment:
why create 9000? I only find that port 8123 has been used.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on PR #9437:
URL: https://github.com/apache/apisix/pull/9437#issuecomment-1550658298
LGTM
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Sn0rt commented on pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "Sn0rt (via GitHub)" <gi...@apache.org>.
Sn0rt commented on PR #9437:
URL: https://github.com/apache/apisix/pull/9437#issuecomment-1552362911
LGTM
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "soulbird (via GitHub)" <gi...@apache.org>.
soulbird commented on PR #9437:
URL: https://github.com/apache/apisix/pull/9437#issuecomment-1550853314
It is better to change the old test cases and use the real clickhouse sever
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shreemaan-abhishek commented on pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on PR #9437:
URL: https://github.com/apache/apisix/pull/9437#issuecomment-1556766724
@monkeyDluffy6017 I added this commit, isn't it sufficient? https://github.com/apache/apisix/pull/9437/commits/ac3af865ec5cbad0d266d5839d3868b51725073d
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 merged pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 merged PR #9437:
URL: https://github.com/apache/apisix/pull/9437
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shreemaan-abhishek commented on pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on PR #9437:
URL: https://github.com/apache/apisix/pull/9437#issuecomment-1554432025
@Revolyssup, we use vector for collecting logs via syslog. Since Clickhouse is a database, vector does not make sense here.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #9437:
URL: https://github.com/apache/apisix/pull/9437#issuecomment-1556402388
> It is better to change the old test cases and use the real clickhouse sever
Hi @shreemaan-abhishek, do you notice the review comment?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Sn0rt commented on a diff in pull request #9437: chore(ci): use real clickhouse server in ci
Posted by "Sn0rt (via GitHub)" <gi...@apache.org>.
Sn0rt commented on code in PR #9437:
URL: https://github.com/apache/apisix/pull/9437#discussion_r1188149601
##########
ci/pod/docker-compose.plugin.yml:
##########
@@ -324,10 +324,21 @@ services:
networks:
vector_net:
+ clickhouse:
+ image: clickhouse/clickhouse-server:23.4.2-alpine
+ container_name: clickhouse
+ ports:
+ - '8123:8123'
+ - '9000:9000'
Review Comment:
one PR only for one thing I think.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org