Skip to content

Conversation

@keith-turner
Copy link
Contributor

No description provided.

@keith-turner keith-turner added this to the 2.1.5 milestone Jan 14, 2026
@keith-turner
Copy link
Contributor Author

keith-turner commented Jan 14, 2026

Looked at the other code in ManagerClientServiceHandler that handles setting system,namespace, and table props and those all seem to log exceptions.

Map.of(property, value));
}
} catch (IllegalStateException ex) {
log.warn("Invalid table property, tried to set: tableId: " + tableId.canonical() + " to: "
Copy link
Contributor

Choose a reason for hiding this comment

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

Line 574 can also return an IllegalStateException

PropUtil.removeProperties(manager.getContext(),
            TablePropKey.of(manager.getContext(), tableId), List.of(property));

This log message should include the table OP instead of assuming it's always a set opt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in 5480b83

PropUtil.setProperties(manager.getContext(), TablePropKey.of(manager.getContext(), tableId),
Map.of(property, value));
} else {
throw new UnsupportedOperationException("table operation:" + op.name());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently the client code is only calling the RPC w/ the two values. However this server side code should not ignore seeing something else. This new throw should cause an error to be logged and TApplicationException on the client side should this ever happen.

@keith-turner keith-turner merged commit 6f9aba4 into apache:2.1 Jan 14, 2026
8 checks passed
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.

2 participants