You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "jacek-lewandowski (via GitHub)" <gi...@apache.org> on 2023/09/06 13:59:22 UTC
[GitHub] [cassandra] jacek-lewandowski opened a new pull request, #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
jacek-lewandowski opened a new pull request, #2672:
URL: https://github.com/apache/cassandra/pull/2672
(no 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: pr-unsubscribe@cassandra.apache.org
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 #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "bereng (via GitHub)" <gi...@apache.org>.
bereng commented on PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#issuecomment-1715157382
Right I see you reverted all PRs. +1 though I still dislike the single utility class... :shrug:
--
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: pr-unsubscribe@cassandra.apache.org
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] JeremiahDJordan commented on a diff in pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "JeremiahDJordan (via GitHub)" <gi...@apache.org>.
JeremiahDJordan commented on code in PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#discussion_r1318649336
##########
test/distributed/org/apache/cassandra/distributed/util/Auth.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.util;
+
+import org.apache.cassandra.auth.CassandraRoleManager;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.awaitility.Awaitility.await;
+
+public class Auth
+{
+
+ public static void waitForExistingRoles(IInvokableInstance instance)
Review Comment:
No existing utility classes this can fit in? That would be preferable to me, more chance of others finding and using it rather than making their own version of it, if it is in a common location.
--
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: pr-unsubscribe@cassandra.apache.org
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] jacek-lewandowski commented on pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#issuecomment-1715121000
Reverted to the `Auth` approach.
--
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: pr-unsubscribe@cassandra.apache.org
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] jacek-lewandowski closed pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski closed pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
URL: https://github.com/apache/cassandra/pull/2672
--
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: pr-unsubscribe@cassandra.apache.org
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] JeremiahDJordan commented on a diff in pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "JeremiahDJordan (via GitHub)" <gi...@apache.org>.
JeremiahDJordan commented on code in PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#discussion_r1318646925
##########
test/unit/org/apache/cassandra/ServerTestUtils.java:
##########
@@ -25,6 +25,10 @@
import java.util.Set;
import java.util.stream.Collectors;
+import org.apache.cassandra.auth.CassandraRoleManager;
Review Comment:
No changes needed to this file now?
##########
test/distributed/org/apache/cassandra/distributed/util/Auth.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.util;
+
+import org.apache.cassandra.auth.CassandraRoleManager;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.awaitility.Awaitility.await;
+
+public class Auth
+{
+
+ public static void waitForExistingRoles(IInvokableInstance instance)
Review Comment:
No existing utility classes this can fit in? That would be preferably to me, more chance of others finding and using it rather than making their own version of it, if it is in a common location.
--
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: pr-unsubscribe@cassandra.apache.org
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] JeremiahDJordan commented on a diff in pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "JeremiahDJordan (via GitHub)" <gi...@apache.org>.
JeremiahDJordan commented on code in PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#discussion_r1319923843
##########
test/distributed/org/apache/cassandra/distributed/util/Auth.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.util;
+
+import org.apache.cassandra.auth.CassandraRoleManager;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.awaitility.Awaitility.await;
+
+public class Auth
+{
+
+ public static void waitForExistingRoles(IInvokableInstance instance)
Review Comment:
“waitForAuthInitialization” seems very reasonable to be on the cluster object actually? Or possibly even have cluster start by default not return until that has happened if auth was enabled?
just trying to figure out how we avoid every test that gets added in the futures and uses auth from being flaky until a month later when someone remembers “oh yeah, you have auth you have to add this extra wait call”
--
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: pr-unsubscribe@cassandra.apache.org
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] jacek-lewandowski commented on pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#issuecomment-1715121391
@bereng can we merge that?
--
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: pr-unsubscribe@cassandra.apache.org
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 #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "bereng (via GitHub)" <gi...@apache.org>.
bereng commented on PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#issuecomment-1709467842
Can't you create `waitForExistingRoles` in `CQLTester` i.e. and then it can be called from everywhere? That would remove the new single method class for jvm and it could be reused from junits 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: pr-unsubscribe@cassandra.apache.org
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] jacek-lewandowski commented on pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#issuecomment-1709922136
Indeed I forgot about setting the delay to 0 for unit tests. Fixed now.
I'll not move that method outside of the distributed "module" because it applies only to the in-jvm distributed tests, where the delay is non-zero. I'd like to not modify the delay for the distributed tests as they usually start multiple nodes and we may fall into another category of problems.
Repeated tests on CircleCI makes zero sense to me in this case because:
1. the problem has not popped up on CircleCI, so why would it now?
2. I found the problem and was able to reproduce it reliably - then after applying a fix, the problem went away
--
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: pr-unsubscribe@cassandra.apache.org
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] JeremiahDJordan commented on a diff in pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "JeremiahDJordan (via GitHub)" <gi...@apache.org>.
JeremiahDJordan commented on code in PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#discussion_r1319923843
##########
test/distributed/org/apache/cassandra/distributed/util/Auth.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.util;
+
+import org.apache.cassandra.auth.CassandraRoleManager;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.awaitility.Awaitility.await;
+
+public class Auth
+{
+
+ public static void waitForExistingRoles(IInvokableInstance instance)
Review Comment:
“waitForAuthInitialization” seems very reasonable to be on the cluster object actually? Or possibly even have cluster start by default not return until that has happened if auth was enabled?
just trying to figure out how we avoid every test that gets added in the futures and uses auth from being flaky until a month later when someone remembers “oh you bit you have auth you have to add this extra wait call”
--
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: pr-unsubscribe@cassandra.apache.org
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] JeremiahDJordan commented on a diff in pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "JeremiahDJordan (via GitHub)" <gi...@apache.org>.
JeremiahDJordan commented on code in PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#discussion_r1319923843
##########
test/distributed/org/apache/cassandra/distributed/util/Auth.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.util;
+
+import org.apache.cassandra.auth.CassandraRoleManager;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.awaitility.Awaitility.await;
+
+public class Auth
+{
+
+ public static void waitForExistingRoles(IInvokableInstance instance)
Review Comment:
“waitForAuthInitialization” seems very reasonable to be on the cluster object actually? Or possibly even have cluster start by default not return until that has happened if auth was enabled?
##########
test/distributed/org/apache/cassandra/distributed/util/Auth.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.util;
+
+import org.apache.cassandra.auth.CassandraRoleManager;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.awaitility.Awaitility.await;
+
+public class Auth
+{
+
+ public static void waitForExistingRoles(IInvokableInstance instance)
Review Comment:
“waitForAuthInitialization” seems very reasonable to be on the cluster object actually? Or possibly even have cluster start by default not return until that has happened if auth was enabled?
--
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: pr-unsubscribe@cassandra.apache.org
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] jacek-lewandowski commented on a diff in pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on code in PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#discussion_r1321137688
##########
test/distributed/org/apache/cassandra/distributed/util/Auth.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.util;
+
+import org.apache.cassandra.auth.CassandraRoleManager;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.awaitility.Awaitility.await;
+
+public class Auth
+{
+
+ public static void waitForExistingRoles(IInvokableInstance instance)
Review Comment:
moved to `Cluster`. I don't want to call it automatically because that would introduce some implicit semantic. I didn't really investigated what those tests are doing...
--
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: pr-unsubscribe@cassandra.apache.org
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 diff in pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "bereng (via GitHub)" <gi...@apache.org>.
bereng commented on code in PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#discussion_r1322406363
##########
test/distributed/org/apache/cassandra/distributed/util/Auth.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.util;
+
+import org.apache.cassandra.auth.CassandraRoleManager;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.awaitility.Awaitility.await;
+
+public class Auth
+{
+
+ public static void waitForExistingRoles(IInvokableInstance instance)
Review Comment:
`SSTableGeneratorTest` in the distributed package already uses `CQLTester` so it wouldn't be the first case...
--
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: pr-unsubscribe@cassandra.apache.org
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] jacek-lewandowski commented on a diff in pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on code in PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#discussion_r1318990591
##########
test/unit/org/apache/cassandra/ServerTestUtils.java:
##########
@@ -25,6 +25,10 @@
import java.util.Set;
import java.util.stream.Collectors;
+import org.apache.cassandra.auth.CassandraRoleManager;
Review Comment:
indeed, that would be caught by checkstyle anyway
--
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: pr-unsubscribe@cassandra.apache.org
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] jacek-lewandowski commented on pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#issuecomment-1710616568
@bereng I'll obviously run the CI
--
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: pr-unsubscribe@cassandra.apache.org
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 #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "bereng (via GitHub)" <gi...@apache.org>.
bereng commented on PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#issuecomment-1710188740
- Does `ServerTestUtils.java` have unsued imports now?
- The change in `CQLTester` impacts 90% of junits so a CI run is mandatory here imo.
- The repeated runs are not optional AFAIK so we'll need them. Do a 100 or 500 if you're confident enough...
--
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: pr-unsubscribe@cassandra.apache.org
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] jacek-lewandowski commented on a diff in pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on code in PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#discussion_r1318990116
##########
test/distributed/org/apache/cassandra/distributed/util/Auth.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.util;
+
+import org.apache.cassandra.auth.CassandraRoleManager;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.awaitility.Awaitility.await;
+
+public class Auth
+{
+
+ public static void waitForExistingRoles(IInvokableInstance instance)
Review Comment:
`CQLTester` is unrelated to in-jvm dtests. There is no utility class in distributed in-jvm tests. I can only move it either Cluster or Instance implementation
--
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: pr-unsubscribe@cassandra.apache.org
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] jacek-lewandowski commented on pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#issuecomment-1711554643
- [ ] trunk [j11](https://app.circleci.com/pipelines/github/jacek-lewandowski/cassandra/868/workflows/1a65d5e7-c3ea-415f-bf6a-eb7048775d22)
- [ ] 5.0 [j11](https://app.circleci.com/pipelines/github/jacek-lewandowski/cassandra/870/workflows/b9a4d2b6-1d42-4f02-a071-73f463e0094f)
- [ ] 4.1 [j8](https://app.circleci.com/pipelines/github/jacek-lewandowski/cassandra/869/workflows/edae89f2-9374-46b3-9e12-c8412771e325)
- [ ] 4.1 [j11](https://app.circleci.com/pipelines/github/jacek-lewandowski/cassandra/869/workflows/aa17d4f6-772e-4d94-92c0-66354106f51c)
--
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: pr-unsubscribe@cassandra.apache.org
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] jacek-lewandowski commented on pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#issuecomment-1715178896
Me too @bereng, believe me.
Thanks for review!
--
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: pr-unsubscribe@cassandra.apache.org
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] jacek-lewandowski commented on pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#issuecomment-1713386491
Rerun dtest after moving the method https://app.circleci.com/pipelines/github/jacek-lewandowski/cassandra/873/workflows/f5346b96-0650-4af6-8205-2139d37f784b
--
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: pr-unsubscribe@cassandra.apache.org
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] jacek-lewandowski commented on pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#issuecomment-1713453428
@JeremiahDJordan @bereng It seems like I cannot move that method to `Cluster` as we get class loader problems.
--
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: pr-unsubscribe@cassandra.apache.org
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 #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "bereng (via GitHub)" <gi...@apache.org>.
bereng commented on PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#issuecomment-1715002640
Ok the current PRs seem to all have the code moved into `Cluster`. You need to push new ones yes?
Just for reference the old CI runs you have previous to the move [here](https://github.com/apache/cassandra/pull/2672#issuecomment-1711554643) are fine. You're hitiing c16778 and c18360 on trunk
--
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: pr-unsubscribe@cassandra.apache.org
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] jacek-lewandowski commented on a diff in pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on code in PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#discussion_r1322544404
##########
test/distributed/org/apache/cassandra/distributed/util/Auth.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.util;
+
+import org.apache.cassandra.auth.CassandraRoleManager;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.awaitility.Awaitility.await;
+
+public class Auth
+{
+
+ public static void waitForExistingRoles(IInvokableInstance instance)
Review Comment:
This is not a good example. That test is a fuzz test implemented with Harry and has nothing to do with in-jvm dtests as far as I can see (except being under the `distributed` root)
--
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: pr-unsubscribe@cassandra.apache.org
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] JeremiahDJordan commented on pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "JeremiahDJordan (via GitHub)" <gi...@apache.org>.
JeremiahDJordan commented on PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#issuecomment-1708550682
Should the auth creation wait time be lowered as well to improve test times?
--
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: pr-unsubscribe@cassandra.apache.org
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 diff in pull request #2672: CASSANDRA-18821: Fix too early attempts to connect when auth is enabled
Posted by "bereng (via GitHub)" <gi...@apache.org>.
bereng commented on code in PR #2672:
URL: https://github.com/apache/cassandra/pull/2672#discussion_r1318806937
##########
test/distributed/org/apache/cassandra/distributed/util/Auth.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.util;
+
+import org.apache.cassandra.auth.CassandraRoleManager;
+import org.apache.cassandra.distributed.api.IInvokableInstance;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.awaitility.Awaitility.await;
+
+public class Auth
+{
+
+ public static void waitForExistingRoles(IInvokableInstance instance)
Review Comment:
`CQLTester` imo is a good candidate. My 2cts.
--
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: pr-unsubscribe@cassandra.apache.org
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