You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "keith-turner (via GitHub)" <gi...@apache.org> on 2025/09/22 22:06:21 UTC
[PR] avoid setting table props in site config in VolumeChooserIT [accumulo]
keith-turner opened a new pull request, #5918:
URL: https://github.com/apache/accumulo/pull/5918
Moved setting table props from site config to namespace config as needed in VolumeChooserIT. Most test methods were already setting the props that were set in site config. This is follow up to #5886
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] avoid setting table props in site config in VolumeChooserIT [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #5918:
URL: https://github.com/apache/accumulo/pull/5918#discussion_r2370660971
##########
test/src/main/java/org/apache/accumulo/test/VolumeChooserIT.java:
##########
@@ -299,19 +291,23 @@ public void twoTablesRandomVolumeChooser() throws Exception {
// Create namespace
try (AccumuloClient client = Accumulo.newClient().from(getClientProperties()).build()) {
- createAndVerify(client, namespace1, v1 + "," + v2 + "," + v3);
- createAndVerify(client, namespace2, v1 + "," + v2 + "," + v3);
+ createAndVerify(client, namespace1, v1 + "", v1 + "," + v2 + "," + v3);
+ createAndVerify(client, namespace2, v2 + "", v1 + "," + v2 + "," + v3);
}
}
- private void createAndVerify(AccumuloClient client, String ns, String expectedVolumes)
- throws Exception {
+ private void createAndVerify(AccumuloClient client, String ns, String preferred,
+ String expectedVolumes) throws Exception {
client.namespaceOperations().create(ns);
// Set properties on the namespace
client.namespaceOperations().setProperty(ns, PERTABLE_CHOOSER_PROP,
RandomVolumeChooser.class.getName());
+ // The random volume chooser should not use this property, so setting it should not cause a
+ // problem
+ client.namespaceOperations().setProperty(ns, PREFERRED_CHOOSER_PROP, preferred);
+
Review Comment:
I think the attempt to reuse this createAndVerify method for different test cases is very confusing. We should just get rid of the method, and do the work in each place to create the namespace with the desired config for each 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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] avoid setting table props in site config in VolumeChooserIT [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner merged PR #5918:
URL: https://github.com/apache/accumulo/pull/5918
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] avoid setting table props in site config in VolumeChooserIT [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on PR #5918:
URL: https://github.com/apache/accumulo/pull/5918#issuecomment-3321644268
Noticed #5917 while working on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] avoid setting table props in site config in VolumeChooserIT [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #5918:
URL: https://github.com/apache/accumulo/pull/5918#discussion_r2370872020
##########
test/src/main/java/org/apache/accumulo/test/VolumeChooserIT.java:
##########
@@ -299,19 +291,23 @@ public void twoTablesRandomVolumeChooser() throws Exception {
// Create namespace
try (AccumuloClient client = Accumulo.newClient().from(getClientProperties()).build()) {
- createAndVerify(client, namespace1, v1 + "," + v2 + "," + v3);
- createAndVerify(client, namespace2, v1 + "," + v2 + "," + v3);
+ createAndVerify(client, namespace1, v1 + "", v1 + "," + v2 + "," + v3);
+ createAndVerify(client, namespace2, v2 + "", v1 + "," + v2 + "," + v3);
}
}
- private void createAndVerify(AccumuloClient client, String ns, String expectedVolumes)
- throws Exception {
+ private void createAndVerify(AccumuloClient client, String ns, String preferred,
+ String expectedVolumes) throws Exception {
client.namespaceOperations().create(ns);
// Set properties on the namespace
client.namespaceOperations().setProperty(ns, PERTABLE_CHOOSER_PROP,
RandomVolumeChooser.class.getName());
+ // The random volume chooser should not use this property, so setting it should not cause a
+ // problem
+ client.namespaceOperations().setProperty(ns, PREFERRED_CHOOSER_PROP, preferred);
+
Review Comment:
I started trying to clean up some of this test, and also ran into #5917
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org