You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/09/10 10:23:20 UTC
[GitHub] [cassandra] bereng opened a new pull request #747: CircleCI should run cqlshlib tests
bereng opened a new pull request #747:
URL: https://github.com/apache/cassandra/pull/747
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r509874245
##########
File path: .circleci/config-2_1.yml
##########
@@ -28,6 +28,12 @@ j8_small_par_executor: &j8_small_par_executor
#exec_resource_class: xlarge
parallelism: 1
+j8_small_executor: &j8_small_executor
+ executor:
+ name: java8-executor
+ exec_resource_class: medium
Review comment:
These tests are a super-tiny thing. I would rather be explicit as I _know_ they don't need xlarge rather than relying on defaults.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ekaterinadimitrova2 commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
ekaterinadimitrova2 commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r495412936
##########
File path: .circleci/config-2_1.yml.high_res.patch
##########
@@ -1,34 +1,79 @@
-22,23c22,23
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-28,29c28,29
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 5
-34,35c34,35
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
-40c40
-< #exec_resource_class: xlarge
----
-> exec_resource_class: xlarge
-46,47c46,47
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-52,53c52,53
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
+@@ -19,39 +19,51 @@ default_env_vars: &default_env_vars
+ j8_par_executor: &j8_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+ j8_small_par_executor: &j8_small_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
++ parallelism: 5
++
++j8_small_executor: &j8_small_executor
++ executor:
++ name: java8-executor
++ exec_resource_class: medium
+ parallelism: 1
+
+ j8_medium_par_executor: &j8_medium_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 1
++ exec_resource_class: xlarge
++ parallelism: 2
+
+ j8_seq_executor: &j8_seq_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
+ parallelism: 1 # sequential, single container tests: no parallelism benefits
+
+ j11_par_executor: &j11_par_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+-j11_small_par_executor: &j11_small_par_executor
++j11_small_executor: &j11_small_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: medium
+ parallelism: 1
+
++j11_small_par_executor: &j11_small_par_executor
++ executor:
++ name: java11-executor
++ exec_resource_class: xlarge
++ parallelism: 2
++
Review comment:
It was easier for me to make a clone in my repo.
So looking here, based on your branch I played only with the order of executors, etc
https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:CASSANDRA-16121
I created only the LOWRES and MIDRES, j8_small_executor is better to be introduced first in lowres as it is lowres that gets propagated to all three config options. I know you will ask me then why in the previous update I introduced the large and very large executors in midres - the response is because they are used only there for our needs.
Hope now it is more clear. Feel free to cherry pick from my branch if you agree with me.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r517800808
##########
File path: .circleci/config-2_1.yml
##########
@@ -866,6 +898,23 @@ commands:
path: /tmp/cassandra/build/test/logs
destination: logs
+ run_cqlshlib_tests:
+ parameters:
+ no_output_timeout:
+ type: string
+ default: 15m
+ steps:
+ - run:
+ name: Run cqlshlib Unit Tests
+ command: |
+ export PATH=$JAVA_HOME/bin:$PATH
+ time mv ~/cassandra /tmp
+ cd /tmp/cassandra/pylib
+ ./cassandra-cqlsh-tests.sh ..
+ no_output_timeout: <<parameters.no_output_timeout>>
+ - store_test_results:
Review comment:
That's 'not possible' atm unfortunately. Those tests are triggered through a sh script that builds a docker/virtual env and everything happens in there. Only the xml unit tests results file is fished out to be able to present test results. Logs and any other artifacts are lost _if_ they existed/are kept, which I haven't checked. We discussed sthg similar already [here](https://github.com/apache/cassandra/pull/747#discussion_r493906036).
We have the same problem in [jenkins](https://ci-cassandra.apache.org/job/Cassandra-trunk-cqlsh-tests/cython=no,jdk=jdk_1.8_latest,label=cassandra/392/console) as in circle where we only have the stdout to check any failures.
Making that work in circle + jenkins + change the sh script etc is out of scope as the ticket is only about running them in circle were they weren't before. I would open a new ticket to improve these tests if we want to but I was not my intention to do so in this ticket.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r495823099
##########
File path: .circleci/config-2_1.yml.high_res.patch
##########
@@ -1,34 +1,79 @@
-22,23c22,23
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-28,29c28,29
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 5
-34,35c34,35
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
-40c40
-< #exec_resource_class: xlarge
----
-> exec_resource_class: xlarge
-46,47c46,47
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-52,53c52,53
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
+@@ -19,39 +19,51 @@ default_env_vars: &default_env_vars
+ j8_par_executor: &j8_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+ j8_small_par_executor: &j8_small_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
++ parallelism: 5
++
++j8_small_executor: &j8_small_executor
++ executor:
++ name: java8-executor
++ exec_resource_class: medium
+ parallelism: 1
+
+ j8_medium_par_executor: &j8_medium_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 1
++ exec_resource_class: xlarge
++ parallelism: 2
+
+ j8_seq_executor: &j8_seq_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
+ parallelism: 1 # sequential, single container tests: no parallelism benefits
+
+ j11_par_executor: &j11_par_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+-j11_small_par_executor: &j11_small_par_executor
++j11_small_executor: &j11_small_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: medium
+ parallelism: 1
+
++j11_small_par_executor: &j11_small_par_executor
++ executor:
++ name: java11-executor
++ exec_resource_class: xlarge
++ parallelism: 2
++
Review comment:
@ekaterinadimitrova2 thx for looking into this but your patch doesn't apply unfortunatelly :-( I think there are 2 patches in [one](https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:CASSANDRA-16121#diff-ba268153edd6228316bb0f7b9e21e600R236) file?
I take it you'd rather have the small executors defined in the original config21.yml and let that propagate? I can do that but the current SHA has been tested low, mid and high. That change would mean retesting that again. Do you feel this is a cosmetic nit or do you feel strongly about it?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r494043063
##########
File path: .circleci/config-2_1.yml.mid_res.patch
##########
@@ -1,8 +1,4 @@
-diff --git .circleci/config-2_1.yml .circleci/config-2_1.yml
Review comment:
As per readme and sh you don't use `git apply` but `patch -o config-2_1.yml.HIGHRES config-2_1.yml config-2_1.yml.high_res.patch` i.e which works perfectly for me.
##########
File path: .circleci/config-2_1.yml
##########
@@ -866,6 +886,23 @@ commands:
path: /tmp/cassandra/build/test/logs
destination: logs
+ run_cqlshlib_tests:
+ parameters:
+ no_output_timeout:
+ type: string
+ default: 15m
+ steps:
+ - run:
+ name: Run cqlshlib Unit Tests
+ command: |
+ export PATH=$JAVA_HOME/bin:$PATH
+ time mv ~/cassandra /tmp
+ cd /tmp/cassandra/pylib
+ ./cassandra-cqlsh-tests.sh ..
+ no_output_timeout: <<parameters.no_output_timeout>>
+ - store_test_results:
+ path: /tmp/cassandra/pylib
Review comment:
I think it is correct. That should point where you want the XML for unit tests circle runs stored. But this being a sh that moves that file around you have to [_collect_](https://circleci.com/docs/2.0/collect-test-data/#metadata-collection-in-custom-test-steps) the results instead of telling circle to _store_ them. So you give him the path where the XML is being generated iiuc. Also notice in the CI runs how this is being collected correctly
##########
File path: .circleci/config-2_1.yml.high_res.patch
##########
@@ -1,34 +1,79 @@
-22,23c22,23
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-28,29c28,29
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 5
-34,35c34,35
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
-40c40
-< #exec_resource_class: xlarge
----
-> exec_resource_class: xlarge
-46,47c46,47
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-52,53c52,53
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
+@@ -19,39 +19,51 @@ default_env_vars: &default_env_vars
+ j8_par_executor: &j8_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+ j8_small_par_executor: &j8_small_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
++ parallelism: 5
++
++j8_small_executor: &j8_small_executor
++ executor:
++ name: java8-executor
++ exec_resource_class: medium
+ parallelism: 1
+
+ j8_medium_par_executor: &j8_medium_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 1
++ exec_resource_class: xlarge
++ parallelism: 2
+
+ j8_seq_executor: &j8_seq_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
+ parallelism: 1 # sequential, single container tests: no parallelism benefits
+
+ j11_par_executor: &j11_par_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+-j11_small_par_executor: &j11_small_par_executor
++j11_small_executor: &j11_small_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: medium
+ parallelism: 1
+
++j11_small_par_executor: &j11_small_par_executor
++ executor:
++ name: java11-executor
++ exec_resource_class: xlarge
++ parallelism: 2
++
Review comment:
I am not following you here :-) on point 3.1 you are dealing with midres but you mention highres...?
I _think_ your concern is with 'adding' `j11_small_par_executor`? If you look a bit higher in the patch you will see this is `diff` noise. Diff gets confused between `j11_small_executor` (the new one) and `j11_small_par_executor`. In fact I am only adding `j11_small_executor` but the diff makes a mess of that.
I am following what I see on the original midres patch i.e. which is where new executors are [added](https://github.com/apache/cassandra/blob/be97d67c0d3bad9e35eb4a5b4476a49e1ef8c935/.circleci/config-2_1.yml.mid_res.patch#L37)
Also if you run `generate.sh` you'll see it runs and files don't change. So everything is in sync imo.
Am I barking up the wrong tree?
##########
File path: .circleci/config-2_1.yml.mid_res.patch
##########
@@ -1,8 +1,4 @@
-diff --git .circleci/config-2_1.yml .circleci/config-2_1.yml
Review comment:
As per readme and sh you don't use `git apply` but `patch -o config-2_1.yml.HIGHRES config-2_1.yml config-2_1.yml.high_res.patch` i.e which works perfectly for me. Can you confirm if it works for you?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r495700388
##########
File path: .circleci/config-2_1.yml.mid_res.patch
##########
@@ -1,8 +1,4 @@
-diff --git .circleci/config-2_1.yml .circleci/config-2_1.yml
Review comment:
np, this patch, diff, apply, change etc is very fiddly :-)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r495937420
##########
File path: .circleci/config-2_1.yml.high_res.patch
##########
@@ -1,34 +1,79 @@
-22,23c22,23
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-28,29c28,29
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 5
-34,35c34,35
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
-40c40
-< #exec_resource_class: xlarge
----
-> exec_resource_class: xlarge
-46,47c46,47
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-52,53c52,53
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
+@@ -19,39 +19,51 @@ default_env_vars: &default_env_vars
+ j8_par_executor: &j8_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+ j8_small_par_executor: &j8_small_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
++ parallelism: 5
++
++j8_small_executor: &j8_small_executor
++ executor:
++ name: java8-executor
++ exec_resource_class: medium
+ parallelism: 1
+
+ j8_medium_par_executor: &j8_medium_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 1
++ exec_resource_class: xlarge
++ parallelism: 2
+
+ j8_seq_executor: &j8_seq_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
+ parallelism: 1 # sequential, single container tests: no parallelism benefits
+
+ j11_par_executor: &j11_par_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+-j11_small_par_executor: &j11_small_par_executor
++j11_small_executor: &j11_small_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: medium
+ parallelism: 1
+
++j11_small_par_executor: &j11_small_par_executor
++ executor:
++ name: java11-executor
++ exec_resource_class: xlarge
++ parallelism: 2
++
Review comment:
weird. I get `patch -o config-2_1.yml.MIDRES config-2_1.yml config-2_1.yml.mid_res.patch
patching file config-2_1.yml.MIDRES (read from config-2_1.yml)
patching file config-2_1.yml.MIDRES (read from config-2_1.yml)
Reversed (or previously applied) patch detected! Assume -R? [n]`
Anyway I can do that change tomorrow
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng closed pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng closed pull request #747:
URL: https://github.com/apache/cassandra/pull/747
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ekaterinadimitrova2 commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
ekaterinadimitrova2 commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r495412471
##########
File path: .circleci/config-2_1.yml
##########
@@ -866,6 +886,23 @@ commands:
path: /tmp/cassandra/build/test/logs
destination: logs
+ run_cqlshlib_tests:
+ parameters:
+ no_output_timeout:
+ type: string
+ default: 15m
+ steps:
+ - run:
+ name: Run cqlshlib Unit Tests
+ command: |
+ export PATH=$JAVA_HOME/bin:$PATH
+ time mv ~/cassandra /tmp
+ cd /tmp/cassandra/pylib
+ ./cassandra-cqlsh-tests.sh ..
+ no_output_timeout: <<parameters.no_output_timeout>>
+ - store_test_results:
+ path: /tmp/cassandra/pylib
Review comment:
I just felt it would be better to store them in separate log directory but ok
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #747:
URL: https://github.com/apache/cassandra/pull/747#issuecomment-714237742
CI [j11](https://app.circleci.com/pipelines/github/bereng/cassandra/155/workflows/25d6895c-d943-4bd3-adae-6bec7f432b2f)
Ci [j8](https://app.circleci.com/pipelines/github/bereng/cassandra/155/workflows/a3a02926-6d2d-4459-b8be-3a6f65762d71)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r509874371
##########
File path: .circleci/config-2_1.yml
##########
@@ -202,6 +217,9 @@ j11_with_dtests_jobs: &j11_with_dtests_jobs
- j11_jvm_dtests:
requires:
- j11_build
+ - j11_cqlshlib_tests:
Review comment:
-1. There is already a ticket I can't find now to remove some of those j11 approvals.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r495700234
##########
File path: .circleci/config-2_1.yml
##########
@@ -866,6 +886,23 @@ commands:
path: /tmp/cassandra/build/test/logs
destination: logs
+ run_cqlshlib_tests:
+ parameters:
+ no_output_timeout:
+ type: string
+ default: 15m
+ steps:
+ - run:
+ name: Run cqlshlib Unit Tests
+ command: |
+ export PATH=$JAVA_HOME/bin:$PATH
+ time mv ~/cassandra /tmp
+ cd /tmp/cassandra/pylib
+ ./cassandra-cqlsh-tests.sh ..
+ no_output_timeout: <<parameters.no_output_timeout>>
+ - store_test_results:
+ path: /tmp/cassandra/pylib
Review comment:
That would imply changing the sh script to post the xml results file to a different folder. That folder would also not be used when tests run locally i.e. So if you don't mind I'd leave it as it is. Feel free to reopen if you change your mind and feel strongly about it.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r495823099
##########
File path: .circleci/config-2_1.yml.high_res.patch
##########
@@ -1,34 +1,79 @@
-22,23c22,23
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-28,29c28,29
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 5
-34,35c34,35
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
-40c40
-< #exec_resource_class: xlarge
----
-> exec_resource_class: xlarge
-46,47c46,47
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-52,53c52,53
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
+@@ -19,39 +19,51 @@ default_env_vars: &default_env_vars
+ j8_par_executor: &j8_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+ j8_small_par_executor: &j8_small_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
++ parallelism: 5
++
++j8_small_executor: &j8_small_executor
++ executor:
++ name: java8-executor
++ exec_resource_class: medium
+ parallelism: 1
+
+ j8_medium_par_executor: &j8_medium_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 1
++ exec_resource_class: xlarge
++ parallelism: 2
+
+ j8_seq_executor: &j8_seq_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
+ parallelism: 1 # sequential, single container tests: no parallelism benefits
+
+ j11_par_executor: &j11_par_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+-j11_small_par_executor: &j11_small_par_executor
++j11_small_executor: &j11_small_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: medium
+ parallelism: 1
+
++j11_small_par_executor: &j11_small_par_executor
++ executor:
++ name: java11-executor
++ exec_resource_class: xlarge
++ parallelism: 2
++
Review comment:
@ekaterinadimitrova2 thx for looking into this but your patch doesn't apply unfortunatelly :-( I think there are 2 patches in [one](https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:CASSANDRA-16121#diff-ba268153edd6228316bb0f7b9e21e600R236) file?
I take it you'd rather have the small executors defined in the original config21.yml and let that propagate? I can do that but the current SHA has been tested low, mid and high. That change would mean retesting that again. Do you feel this is a cosmetic nit or do you feel stronly about it?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r509687010
##########
File path: .circleci/config-2_1.yml
##########
@@ -28,6 +28,12 @@ j8_small_par_executor: &j8_small_par_executor
#exec_resource_class: xlarge
parallelism: 1
+j8_small_executor: &j8_small_executor
+ executor:
+ name: java8-executor
+ exec_resource_class: medium
Review comment:
can we do `#exec_resource_class: xlarge` like the rest? No executor selects medium, they rely on default.
##########
File path: .circleci/config-2_1.yml
##########
@@ -202,6 +217,9 @@ j11_with_dtests_jobs: &j11_with_dtests_jobs
- j11_jvm_dtests:
requires:
- j11_build
+ - j11_cqlshlib_tests:
Review comment:
should we have an approval here? j11 doesn't really run anything without explicit approval, this would run for all commits.
##########
File path: .circleci/config-2_1.yml
##########
@@ -28,6 +28,12 @@ j8_small_par_executor: &j8_small_par_executor
#exec_resource_class: xlarge
parallelism: 1
+j8_small_executor: &j8_small_executor
Review comment:
nit, why not `j8_seq_executor`? it exists and serves the same purpose.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r494043063
##########
File path: .circleci/config-2_1.yml.mid_res.patch
##########
@@ -1,8 +1,4 @@
-diff --git .circleci/config-2_1.yml .circleci/config-2_1.yml
Review comment:
As per readme and sh you don't use `git apply` but `patch -o config-2_1.yml.HIGHRES config-2_1.yml config-2_1.yml.high_res.patch` i.e which works perfectly for me.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ekaterinadimitrova2 commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
ekaterinadimitrova2 commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r495412522
##########
File path: .circleci/config-2_1.yml.mid_res.patch
##########
@@ -1,8 +1,4 @@
-diff --git .circleci/config-2_1.yml .circleci/config-2_1.yml
Review comment:
yes, of course, my bad here. Sorry for the confusion
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ekaterinadimitrova2 commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
ekaterinadimitrova2 commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r495908934
##########
File path: .circleci/config-2_1.yml.high_res.patch
##########
@@ -1,34 +1,79 @@
-22,23c22,23
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-28,29c28,29
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 5
-34,35c34,35
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
-40c40
-< #exec_resource_class: xlarge
----
-> exec_resource_class: xlarge
-46,47c46,47
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-52,53c52,53
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
+@@ -19,39 +19,51 @@ default_env_vars: &default_env_vars
+ j8_par_executor: &j8_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+ j8_small_par_executor: &j8_small_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
++ parallelism: 5
++
++j8_small_executor: &j8_small_executor
++ executor:
++ name: java8-executor
++ exec_resource_class: medium
+ parallelism: 1
+
+ j8_medium_par_executor: &j8_medium_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 1
++ exec_resource_class: xlarge
++ parallelism: 2
+
+ j8_seq_executor: &j8_seq_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
+ parallelism: 1 # sequential, single container tests: no parallelism benefits
+
+ j11_par_executor: &j11_par_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+-j11_small_par_executor: &j11_small_par_executor
++j11_small_executor: &j11_small_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: medium
+ parallelism: 1
+
++j11_small_par_executor: &j11_small_par_executor
++ executor:
++ name: java11-executor
++ exec_resource_class: xlarge
++ parallelism: 2
++
Review comment:
The midres created was able to apply for me... Not sure what is the issue on your end. Maybe you tried the generate.sh script which looks also at the highres and I made a POV. only for midres?
Anyway, yes, the small executors should be there, that's what I meant.
Unfortunately, I think it should be moved. Otherwise we are not consistent and it becomes more confusing in the future for updates. All executors which are propagated in the three configurations and have lowres version are defined in config-2_1.yml
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r517800808
##########
File path: .circleci/config-2_1.yml
##########
@@ -866,6 +898,23 @@ commands:
path: /tmp/cassandra/build/test/logs
destination: logs
+ run_cqlshlib_tests:
+ parameters:
+ no_output_timeout:
+ type: string
+ default: 15m
+ steps:
+ - run:
+ name: Run cqlshlib Unit Tests
+ command: |
+ export PATH=$JAVA_HOME/bin:$PATH
+ time mv ~/cassandra /tmp
+ cd /tmp/cassandra/pylib
+ ./cassandra-cqlsh-tests.sh ..
+ no_output_timeout: <<parameters.no_output_timeout>>
+ - store_test_results:
Review comment:
That's 'not possible' atm unfortunately. Those tests are triggered through a sh script that builds a docker/virtual env and everything happens in there. Only the xml unit tests results file is fished out to be able to present test results. Logs and any other artifacts are lost _if_ they existed/are kept, which I haven't checked. We discussed sthg similar already [here](https://github.com/apache/cassandra/pull/747#discussion_r493906036).
We have the same problem in [jenkins](https://ci-cassandra.apache.org/job/Cassandra-trunk-cqlsh-tests/cython=no,jdk=jdk_1.8_latest,label=cassandra/392/console) as in circle where we only have the stdout to check any failures.
Making that work in circle + jenkins + change the sh script etc is out of scope as the ticket is only about running them in circle were they weren't before. I would open a new ticket to improve these tests if we want to but it was not my intention to do so in this ticket.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ekaterinadimitrova2 commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
ekaterinadimitrova2 commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r495908934
##########
File path: .circleci/config-2_1.yml.high_res.patch
##########
@@ -1,34 +1,79 @@
-22,23c22,23
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-28,29c28,29
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 5
-34,35c34,35
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
-40c40
-< #exec_resource_class: xlarge
----
-> exec_resource_class: xlarge
-46,47c46,47
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-52,53c52,53
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
+@@ -19,39 +19,51 @@ default_env_vars: &default_env_vars
+ j8_par_executor: &j8_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+ j8_small_par_executor: &j8_small_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
++ parallelism: 5
++
++j8_small_executor: &j8_small_executor
++ executor:
++ name: java8-executor
++ exec_resource_class: medium
+ parallelism: 1
+
+ j8_medium_par_executor: &j8_medium_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 1
++ exec_resource_class: xlarge
++ parallelism: 2
+
+ j8_seq_executor: &j8_seq_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
+ parallelism: 1 # sequential, single container tests: no parallelism benefits
+
+ j11_par_executor: &j11_par_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+-j11_small_par_executor: &j11_small_par_executor
++j11_small_executor: &j11_small_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: medium
+ parallelism: 1
+
++j11_small_par_executor: &j11_small_par_executor
++ executor:
++ name: java11-executor
++ exec_resource_class: xlarge
++ parallelism: 2
++
Review comment:
The midres created was able to apply for me... Not sure what is the issue on your end. Maybe you tried the generate.sh script which looks also at the highres and I made a POV. only for midres.
Yes, the small executors should be there, that's what I meant.
Unfortunately, I think it should be moved. Otherwise we are not consistent and it becomes more confusing in the future for updates. All executors which are propagated in the three configurations and have lowres version are defined in config-2_1.yml
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ekaterinadimitrova2 commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
ekaterinadimitrova2 commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r495412936
##########
File path: .circleci/config-2_1.yml.high_res.patch
##########
@@ -1,34 +1,79 @@
-22,23c22,23
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-28,29c28,29
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 5
-34,35c34,35
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
-40c40
-< #exec_resource_class: xlarge
----
-> exec_resource_class: xlarge
-46,47c46,47
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-52,53c52,53
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
+@@ -19,39 +19,51 @@ default_env_vars: &default_env_vars
+ j8_par_executor: &j8_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+ j8_small_par_executor: &j8_small_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
++ parallelism: 5
++
++j8_small_executor: &j8_small_executor
++ executor:
++ name: java8-executor
++ exec_resource_class: medium
+ parallelism: 1
+
+ j8_medium_par_executor: &j8_medium_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 1
++ exec_resource_class: xlarge
++ parallelism: 2
+
+ j8_seq_executor: &j8_seq_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
+ parallelism: 1 # sequential, single container tests: no parallelism benefits
+
+ j11_par_executor: &j11_par_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+-j11_small_par_executor: &j11_small_par_executor
++j11_small_executor: &j11_small_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: medium
+ parallelism: 1
+
++j11_small_par_executor: &j11_small_par_executor
++ executor:
++ name: java11-executor
++ exec_resource_class: xlarge
++ parallelism: 2
++
Review comment:
It was easier for me to make a clone of your work in my repo and test the corrections.
So looking here, based on your branch I played only with the order of executors, etc
https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:CASSANDRA-16121
I created only the LOWRES and MIDRES, j8_small_executor is better to be introduced first in lowres as it is lowres that gets propagated to all three config options. I know you will ask me then why in the previous update I introduced the large and very large executors in midres - the response is because they are used only there for our needs.
Hope now it is more clear. Feel free to cherry pick from my branch if you agree with me.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #747:
URL: https://github.com/apache/cassandra/pull/747#issuecomment-693177501
CI
- [j11](https://app.circleci.com/pipelines/github/bereng/cassandra/111/workflows/28459ff8-05b1-41cc-af19-9aef9016cdb3)
- [j8](https://app.circleci.com/pipelines/github/bereng/cassandra/111/workflows/fe113eb6-4ac1-4e8e-aebf-c4dec442c2ce)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r494045537
##########
File path: .circleci/config-2_1.yml
##########
@@ -866,6 +886,23 @@ commands:
path: /tmp/cassandra/build/test/logs
destination: logs
+ run_cqlshlib_tests:
+ parameters:
+ no_output_timeout:
+ type: string
+ default: 15m
+ steps:
+ - run:
+ name: Run cqlshlib Unit Tests
+ command: |
+ export PATH=$JAVA_HOME/bin:$PATH
+ time mv ~/cassandra /tmp
+ cd /tmp/cassandra/pylib
+ ./cassandra-cqlsh-tests.sh ..
+ no_output_timeout: <<parameters.no_output_timeout>>
+ - store_test_results:
+ path: /tmp/cassandra/pylib
Review comment:
I think it is correct. That should point where you want the XML for unit tests circle runs stored. But this being a sh that moves that file around you have to [_collect_](https://circleci.com/docs/2.0/collect-test-data/#metadata-collection-in-custom-test-steps) the results instead of telling circle to _store_ them. So you give him the path where the XML is being generated iiuc. Also notice in the CI runs how this is being collected correctly
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r509874371
##########
File path: .circleci/config-2_1.yml
##########
@@ -202,6 +217,9 @@ j11_with_dtests_jobs: &j11_with_dtests_jobs
- j11_jvm_dtests:
requires:
- j11_build
+ - j11_cqlshlib_tests:
Review comment:
-1. There is already a ticket I can't find now to remove some j11 approvals.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #747:
URL: https://github.com/apache/cassandra/pull/747#issuecomment-691818108
CI:
- [Does not](https://ci-cassandra.apache.org/view/patches/job/Cassandra-devbranch/15/testReport/) interfere with ci-cass
- [j11](https://app.circleci.com/pipelines/github/bereng/cassandra/107/workflows/7a1b4773-f895-4736-b17d-cd7e88e743bc): All good. New job `j11_cqlshlib_tests` ran with the known failure being reported.
- [j8](https://app.circleci.com/pipelines/github/bereng/cassandra/107/workflows/618406f1-1797-4510-8fe3-2e18645d4d90): All good. New job `j8_cqlshlib_tests` ran with the known failure being reported. The failures are OOM due to xml processing which is a known 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r494043063
##########
File path: .circleci/config-2_1.yml.mid_res.patch
##########
@@ -1,8 +1,4 @@
-diff --git .circleci/config-2_1.yml .circleci/config-2_1.yml
Review comment:
As per readme and sh you don't use `git apply` but `patch -o config-2_1.yml.HIGHRES config-2_1.yml config-2_1.yml.high_res.patch` i.e which works perfectly for me. Can you confirm if it works for you?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r517552633
##########
File path: .circleci/config-2_1.yml
##########
@@ -866,6 +898,23 @@ commands:
path: /tmp/cassandra/build/test/logs
destination: logs
+ run_cqlshlib_tests:
+ parameters:
+ no_output_timeout:
+ type: string
+ default: 15m
+ steps:
+ - run:
+ name: Run cqlshlib Unit Tests
+ command: |
+ export PATH=$JAVA_HOME/bin:$PATH
+ time mv ~/cassandra /tmp
+ cd /tmp/cassandra/pylib
+ ./cassandra-cqlsh-tests.sh ..
+ no_output_timeout: <<parameters.no_output_timeout>>
+ - store_test_results:
Review comment:
can we also store the logs? here is what we do for unit test
```
- store_test_results:
path: /tmp/cassandra/build/test/output/
- store_artifacts:
path: /tmp/cassandra/build/test/output
destination: junitxml
- store_artifacts:
path: /tmp/cassandra/build/test/logs
destination: logs
```
If a test fails, we need a way to to look at the results to debug
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ekaterinadimitrova2 commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
ekaterinadimitrova2 commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r493906036
##########
File path: .circleci/config-2_1.yml
##########
@@ -866,6 +886,23 @@ commands:
path: /tmp/cassandra/build/test/logs
destination: logs
+ run_cqlshlib_tests:
+ parameters:
+ no_output_timeout:
+ type: string
+ default: 15m
+ steps:
+ - run:
+ name: Run cqlshlib Unit Tests
+ command: |
+ export PATH=$JAVA_HOME/bin:$PATH
+ time mv ~/cassandra /tmp
+ cd /tmp/cassandra/pylib
+ ./cassandra-cqlsh-tests.sh ..
+ no_output_timeout: <<parameters.no_output_timeout>>
+ - store_test_results:
+ path: /tmp/cassandra/pylib
Review comment:
Are you sure about the directory?
Other jobs have output/logs/results
##########
File path: .circleci/config-2_1.yml.mid_res.patch
##########
@@ -1,8 +1,4 @@
-diff --git .circleci/config-2_1.yml .circleci/config-2_1.yml
Review comment:
I have the feeling that something went wrong during patch creation.
If you try git apply check you get an error:
error: patch fragment without header at line 1: @@ -19,39 +19,51 @@ default_env_vars: &default_env_vars
##########
File path: .circleci/config-2_1.yml.high_res.patch
##########
@@ -1,34 +1,79 @@
-22,23c22,23
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-28,29c28,29
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 5
-34,35c34,35
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
-40c40
-< #exec_resource_class: xlarge
----
-> exec_resource_class: xlarge
-46,47c46,47
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-52,53c52,53
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
+@@ -19,39 +19,51 @@ default_env_vars: &default_env_vars
+ j8_par_executor: &j8_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+ j8_small_par_executor: &j8_small_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
++ parallelism: 5
++
++j8_small_executor: &j8_small_executor
++ executor:
++ name: java8-executor
++ exec_resource_class: medium
+ parallelism: 1
+
+ j8_medium_par_executor: &j8_medium_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 1
++ exec_resource_class: xlarge
++ parallelism: 2
+
+ j8_seq_executor: &j8_seq_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
+ parallelism: 1 # sequential, single container tests: no parallelism benefits
+
+ j11_par_executor: &j11_par_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+-j11_small_par_executor: &j11_small_par_executor
++j11_small_executor: &j11_small_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: medium
+ parallelism: 1
+
++j11_small_par_executor: &j11_small_par_executor
++ executor:
++ name: java11-executor
++ exec_resource_class: xlarge
++ parallelism: 2
++
Review comment:
Below stuff shouldn't appear here.
+j11_small_par_executor: &j11_small_par_executor
+ executor:
+ name: java11-executor
+ exec_resource_class: xlarge
+ parallelism: 2
The flow is as follows:
1) Because we need to add new jobs, we do that in both config.yml and config-2_1.yml
2) The jobs are added to config.yml and config-2_1.yml with their low res and commit those two files the way the lowres is supposed to look like
3) Then the patch creation comes:
- update the config-2_1.yml the way you want it for midres and create a patch with name config-2_1.yml.mid_res.patch. When you look at the patch, only resources will be changed - from those used for lowres to those used for highres. Save the patch and revert the changes in config-2_1.yml so it stays in the lowres
- repeat the procedure for highres patch
4) Now when you have your patch files, you don't have to do anything else then run generate.sh which will update config.yml.LOWRES, config.yml.MIDRES, config.yml.HIGHRES
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ekaterinadimitrova2 commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
ekaterinadimitrova2 commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r509706201
##########
File path: .circleci/config-2_1.yml
##########
@@ -28,6 +28,12 @@ j8_small_par_executor: &j8_small_par_executor
#exec_resource_class: xlarge
parallelism: 1
+j8_small_executor: &j8_small_executor
Review comment:
It uses different resources which are needed for different tests, that's why we created the new one :-)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #747: CircleCI should run cqlshlib tests
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #747:
URL: https://github.com/apache/cassandra/pull/747#discussion_r494051020
##########
File path: .circleci/config-2_1.yml.high_res.patch
##########
@@ -1,34 +1,79 @@
-22,23c22,23
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-28,29c28,29
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 5
-34,35c34,35
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
-40c40
-< #exec_resource_class: xlarge
----
-> exec_resource_class: xlarge
-46,47c46,47
-< #exec_resource_class: xlarge
-< parallelism: 4
----
-> exec_resource_class: xlarge
-> parallelism: 100
-52,53c52,53
-< #exec_resource_class: xlarge
-< parallelism: 1
----
-> exec_resource_class: xlarge
-> parallelism: 2
+@@ -19,39 +19,51 @@ default_env_vars: &default_env_vars
+ j8_par_executor: &j8_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+ j8_small_par_executor: &j8_small_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
++ parallelism: 5
++
++j8_small_executor: &j8_small_executor
++ executor:
++ name: java8-executor
++ exec_resource_class: medium
+ parallelism: 1
+
+ j8_medium_par_executor: &j8_medium_par_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
+- parallelism: 1
++ exec_resource_class: xlarge
++ parallelism: 2
+
+ j8_seq_executor: &j8_seq_executor
+ executor:
+ name: java8-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: xlarge
+ parallelism: 1 # sequential, single container tests: no parallelism benefits
+
+ j11_par_executor: &j11_par_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
+- parallelism: 4
++ exec_resource_class: xlarge
++ parallelism: 100
+
+-j11_small_par_executor: &j11_small_par_executor
++j11_small_executor: &j11_small_executor
+ executor:
+ name: java11-executor
+- #exec_resource_class: xlarge
++ exec_resource_class: medium
+ parallelism: 1
+
++j11_small_par_executor: &j11_small_par_executor
++ executor:
++ name: java11-executor
++ exec_resource_class: xlarge
++ parallelism: 2
++
Review comment:
I am not following you here :-) on point 3.1 you are dealing with midres but you mention highres...?
I _think_ your concern is with 'adding' `j11_small_par_executor`? If you look a bit higher in the patch you will see this is `diff` noise. Diff gets confused between `j11_small_executor` (the new one) and `j11_small_par_executor`. In fact I am only adding `j11_small_executor` but the diff makes a mess of that.
I am following what I see on the original midres patch i.e. which is where new executors are [added](https://github.com/apache/cassandra/blob/be97d67c0d3bad9e35eb4a5b4476a49e1ef8c935/.circleci/config-2_1.yml.mid_res.patch#L37)
Also if you run `generate.sh` you'll see it runs and files don't change. So everything is in sync imo.
Am I barking up the wrong tree?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org