You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "richochetclementine1315 (via GitHub)" <gi...@apache.org> on 2025/01/17 20:13:21 UTC
[PR] Renamed TxName to FateOp [accumulo]
richochetclementine1315 opened a new pull request, #5273:
URL: https://github.com/apache/accumulo/pull/5273
Tried to solve issue #5230 .... Renamed TxName to FateOp.
--
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] Renamed TxName to FateOp [accumulo]
Posted by "kevinrr888 (via GitHub)" <gi...@apache.org>.
kevinrr888 commented on PR #5273:
URL: https://github.com/apache/accumulo/pull/5273#issuecomment-2610116860
If the project is in a state where it cannot compile, which it currently is in this PR, then the command won't work. Ensure all variables and methods changed are also changed everywhere they are referenced.
--
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] Renamed TxName to FateOp [accumulo]
Posted by "kevinrr888 (via GitHub)" <gi...@apache.org>.
kevinrr888 commented on PR #5273:
URL: https://github.com/apache/accumulo/pull/5273#issuecomment-2605668269
This PR has build failures: https://github.com/apache/accumulo/actions/runs/12890793516/job/35955491442?pr=5273
please ensure all variables or methods changed are also changed everywhere they are referenced.
Also see https://github.com/apache/accumulo/pull/5273#discussion_r1920727542
--
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] Renamed TxName to FateOp [accumulo]
Posted by "kevinrr888 (via GitHub)" <gi...@apache.org>.
kevinrr888 commented on PR #5273:
URL: https://github.com/apache/accumulo/pull/5273#issuecomment-2634648804
Closing due to inactivity and several unaddressed issues and comments
--
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] Renamed TxName to FateOp [accumulo]
Posted by "kevinrr888 (via GitHub)" <gi...@apache.org>.
kevinrr888 commented on code in PR #5273:
URL: https://github.com/apache/accumulo/pull/5273#discussion_r1923827817
##########
server/base/src/main/java/org/apache/accumulo/server/util/fateCommand/FateTxnDetails.java:
##########
@@ -103,8 +103,8 @@ public long getRunning() {
return running;
}
- public String getTxName() {
- return txName;
+ public String getfateOp() {
Review Comment:
```suggestion
public String getFateOp() {
```
##########
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/MetaFateStore.java:
##########
@@ -223,7 +223,7 @@ private void seedTransaction(Fate.FateOperation txName, Repo<T> repo, boolean au
if (autoCleanUp) {
txStore.setTransactionInfo(TxInfo.AUTO_CLEAN, autoCleanUp);
}
- txStore.setTransactionInfo(TxInfo.TX_NAME, txName);
+ txStore.setTransactionInfo(TxInfo.TX_NAME, fateOp);
Review Comment:
ensure that TxInfo.TX_NAME is no longer referenced as it no longer exists. This, any maybe elsewhere, needs to be changed to
`TxInfo.FATE_OP`
##########
server/base/src/main/java/org/apache/accumulo/server/util/fateCommand/FateTxnDetails.java:
##########
@@ -73,8 +73,8 @@ public FateTxnDetails(final long reportTime, final AdminUtil.TransactionStatus t
if (txnStatus.getTop() != null) {
step = txnStatus.getTop();
}
- if (txnStatus.getTxName() != null) {
- txName = txnStatus.getTxName().name();
+ if (txnStatus.getfateOp() != null) {
+ fateOp= txnStatus.getfateOp().name();
Review Comment:
Run the maven command:
`mvn clean package -DskipTests`
to quickly format the project
##########
core/src/main/java/org/apache/accumulo/core/fate/user/schema/FateSchema.java:
##########
@@ -44,8 +44,8 @@ public static class TxInfoColumnFamily {
public static final String STR_NAME = "txinfo";
public static final Text NAME = new Text(STR_NAME);
- public static final String TX_NAME = "txname";
- public static final ColumnFQ TX_NAME_COLUMN = new ColumnFQ(NAME, new Text(TX_NAME));
+ public static final String FATE_OP = "fateOp";
+ public static final ColumnFQ TX_NAME_COLUMN = new ColumnFQ(NAME, new Text(FATE_OP));
Review Comment:
this still needs to be changed
```suggestion
public static final ColumnFQ FATE_OP_COLUMN = new ColumnFQ(NAME, new Text(FATE_OP));
```
--
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] Renamed TxName to FateOp [accumulo]
Posted by "richochetclementine1315 (via GitHub)" <gi...@apache.org>.
richochetclementine1315 commented on PR #5273:
URL: https://github.com/apache/accumulo/pull/5273#issuecomment-2608043077
@kevinrr888 mvn clean package -DskipTests , when running this command its showing error
--
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] Renamed TxName to FateOp [accumulo]
Posted by "kevinrr888 (via GitHub)" <gi...@apache.org>.
kevinrr888 commented on code in PR #5273:
URL: https://github.com/apache/accumulo/pull/5273#discussion_r1920727542
##########
core/src/main/java/org/apache/accumulo/core/fate/user/schema/FateSchema.java:
##########
@@ -44,7 +44,7 @@ public static class TxInfoColumnFamily {
public static final String STR_NAME = "txinfo";
public static final Text NAME = new Text(STR_NAME);
- public static final String TX_NAME = "txname";
+ public static final String TX_NAME = "FateOp";
Review Comment:
```suggestion
public static final String FATE_OP = "fateop";
```
Would be nice to change TX_NAME here to FATE_OP and maintain lowercase string value for consistency
##########
core/src/main/java/org/apache/accumulo/core/fate/AdminUtil.java:
##########
@@ -74,20 +74,20 @@ public static class TransactionStatus {
private final FateId fateId;
private final FateInstanceType instanceType;
private final TStatus status;
- private final Fate.FateOperation txName;
+ private final Fate.FateOperation FateOp;
Review Comment:
Commenting once but applies other places: should maintain camel case for vars. `FateOp` -> `fateOp` would be good
##########
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/MetaFateStore.java:
##########
@@ -223,7 +223,7 @@ private void seedTransaction(Fate.FateOperation txName, Repo<T> repo, boolean au
if (autoCleanUp) {
txStore.setTransactionInfo(TxInfo.AUTO_CLEAN, autoCleanUp);
}
- txStore.setTransactionInfo(TxInfo.TX_NAME, txName);
+ txStore.setTransactionInfo(TxInfo.TX_NAME, FateOp);
Review Comment:
The TxInfo enum value TX_NAME should also be changed to FATE_OP
##########
core/src/main/java/org/apache/accumulo/core/fate/user/schema/FateSchema.java:
##########
@@ -44,7 +44,7 @@ public static class TxInfoColumnFamily {
public static final String STR_NAME = "txinfo";
public static final Text NAME = new Text(STR_NAME);
- public static final String TX_NAME = "txname";
+ public static final String TX_NAME = "FateOp";
public static final ColumnFQ TX_NAME_COLUMN = new ColumnFQ(NAME, new Text(TX_NAME));
Review Comment:
This would also need to be changed to no longer reference TX_NAME
--
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] Renamed TxName to FateOp [accumulo]
Posted by "richochetclementine1315 (via GitHub)" <gi...@apache.org>.
richochetclementine1315 commented on PR #5273:
URL: https://github.com/apache/accumulo/pull/5273#issuecomment-2599709273
@kevinrr888 PTAL :)
--
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] Renamed TxName to FateOp [accumulo]
Posted by "kevinrr888 (via GitHub)" <gi...@apache.org>.
kevinrr888 closed pull request #5273: Renamed TxName to FateOp
URL: https://github.com/apache/accumulo/pull/5273
--
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] Renamed TxName to FateOp [accumulo]
Posted by "kevinrr888 (via GitHub)" <gi...@apache.org>.
kevinrr888 commented on PR #5273:
URL: https://github.com/apache/accumulo/pull/5273#issuecomment-2612943704
`mvn clean package -DskipTests` still needs to be run. Run the command and commit the changes. That command will format the project and should be run before any further commits.
--
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] Renamed TxName to FateOp [accumulo]
Posted by "kevinrr888 (via GitHub)" <gi...@apache.org>.
kevinrr888 commented on code in PR #5273:
URL: https://github.com/apache/accumulo/pull/5273#discussion_r1923832857
##########
core/src/main/java/org/apache/accumulo/core/fate/user/schema/FateSchema.java:
##########
@@ -44,7 +44,7 @@ public static class TxInfoColumnFamily {
public static final String STR_NAME = "txinfo";
public static final Text NAME = new Text(STR_NAME);
- public static final String TX_NAME = "txname";
+ public static final String TX_NAME = "FateOp";
Review Comment:
this still needs to be resolved
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Renamed TxName to FateOp [accumulo]
Posted by "richochetclementine1315 (via GitHub)" <gi...@apache.org>.
richochetclementine1315 commented on PR #5273:
URL: https://github.com/apache/accumulo/pull/5273#issuecomment-2605121734
@kevinrr888 PTAL...
--
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