You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2022/06/29 02:05:27 UTC

[GitHub] [incubator-kvrocks] tisonkun opened a new pull request, #689: ci: refactor CI yaml settings

tisonkun opened a new pull request, #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689

   Sorting out steps and run redis-cli IT cases also on macos.
   
   Signed-off-by: tison <wa...@gmail.com>


-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689#discussion_r909182689


##########
tests/tcl/tests/test_helper.tcl:
##########
@@ -60,6 +60,7 @@ set ::all_tests {
     unit/scripting
     integration/slotmigrate
     integration/slotimport
+    integration/redis-cli

Review Comment:
   OK. I can see the related code. Let me push a new commit.



-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689#issuecomment-1169494248

   Push 62c2952b8a8792bedfb308ba278303085b81c629 to ensure that cache logic works.


-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689#discussion_r909175461


##########
tests/tcl/tests/test_helper.tcl:
##########
@@ -60,6 +60,7 @@ set ::all_tests {
     unit/scripting
     integration/slotmigrate
     integration/slotimport
+    integration/redis-cli

Review Comment:
   Maybe we need change configuration of daily CI since `integration/redis-cli` is mandatory now? e.g. install redis-cli.



-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689#issuecomment-1169614450

   Another thing is that since we build the nightly version of the docker image we can automatically publish it to some registries like the docker hub.


-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689#discussion_r909175461


##########
tests/tcl/tests/test_helper.tcl:
##########
@@ -60,6 +60,7 @@ set ::all_tests {
     unit/scripting
     integration/slotmigrate
     integration/slotimport
+    integration/redis-cli

Review Comment:
   Maybe we need change configuration of daily CI since `integration/redis-cli` is mandatory now?



-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689#issuecomment-1169620714

   Let's move the discussion of reviewing build docker image job to #691.
   
   I'll merge this patch now.


-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] git-hulk commented on pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689#issuecomment-1169619824

   > Another thing is that since we build the nightly version of the docker image we can automatically publish it to some registries like the docker hub, e.g. some tags like `nightly`, `nightly-20220629`.
   > 
   > And I also think building docker image in CI triggered by PRs is unnecessary (maybe an arm evironment is useful, but a docker environment is not necessary for build and test).
   
   Good point that we can push the `nightly` docker image, latest image need to wait for the new release is too slow for most users.


-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun merged pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
tisonkun merged PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689


-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689#issuecomment-1169609906

   > @git-hulk @ShooterIT @PragmaTwice Perhaps we can move build and test tasks from daily CI to per PR CI. They take less than half hour to finish. While building docker image takes more than 2 hours to finish and I'm curious why.
   
   Maybe it is due to that we use QEMU to simulate an arm64 environment.


-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689#issuecomment-1169504372

   Greate changes.
   
   When I was changing this before, I wondered whether `apt install redis` / `brew install redis` could work well as building from source as current way, but i didn't try.


-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689#discussion_r909134586


##########
.github/workflows/kvrocks.yaml:
##########
@@ -15,7 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 
-name: kvrocks ci actions  # don't edit while the badge was depend on this

Review Comment:
   The badge now depends on file name, so it's OK to change this field.



-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689#discussion_r909182309


##########
tests/tcl/tests/test_helper.tcl:
##########
@@ -60,6 +60,7 @@ set ::all_tests {
     unit/scripting
     integration/slotmigrate
     integration/slotimport
+    integration/redis-cli

Review Comment:
   I don't check logics in daily CI. How can it be related?



-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689#issuecomment-1169606788

   @git-hulk @ShooterIT @PragmaTwice Perhaps we can move build and test tasks from daily CI to per PR CI. They take less than half hour to finish. While building docker image takes more than 2 hours to finish and I'm curious why.


-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on pull request #689: ci: refactor CI yaml settings

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #689:
URL: https://github.com/apache/incubator-kvrocks/pull/689#issuecomment-1169611332

   @PragmaTwice thanks for your input. Another approach is still run more Build and Test tasks every day but separate docker job into another workflow.


-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org