Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renamed TxName to FateOp #5273

Conversation

richochetclementine1315

Tried to solve issue #5230 .... Renamed TxName to FateOp.

@@ -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";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static final String TX_NAME = "FateOp";
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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this still needs to be resolved

@@ -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));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would also need to be changed to no longer reference TX_NAME

@@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TxInfo enum value TX_NAME should also be changed to FATE_OP

@richochetclementine1315
Copy link
Author

@kevinrr888 PTAL :)

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));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this still needs to be changed

Suggested change
public static final ColumnFQ TX_NAME_COLUMN = new ColumnFQ(NAME, new Text(FATE_OP));
public static final ColumnFQ FATE_OP_COLUMN = new ColumnFQ(NAME, new Text(FATE_OP));

@@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (txnStatus.getTxName() != null) {
txName = txnStatus.getTxName().name();
if (txnStatus.getfateOp() != null) {
fateOp= txnStatus.getfateOp().name();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run the maven command:
mvn clean package -DskipTests
to quickly format the project

@@ -103,8 +103,8 @@ public long getRunning() {
return running;
}

public String getTxName() {
return txName;
public String getfateOp() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public String getfateOp() {
public String getFateOp() {

@richochetclementine1315
Copy link
Author

@kevinrr888 PTAL...

@kevinrr888
Copy link
Member

kevinrr888 commented Jan 21, 2025

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 #5273 (comment) and #5273 (comment)

@ctubbsii ctubbsii linked an issue Jan 21, 2025 that may be closed by this pull request
@ctubbsii ctubbsii added this to the 4.0.0 milestone Jan 21, 2025
@richochetclementine1315
Copy link
Author

@kevinrr888 mvn clean package -DskipTests , when running this command its showing error

@kevinrr888
Copy link
Member

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.

@kevinrr888
Copy link
Member

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.

@kevinrr888
Copy link
Member

Closing due to inactivity and several unaddressed issues and comments

@kevinrr888 kevinrr888 closed this Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(Trivial) Rename TxName
3 participants