Skip to content

MINOR: correct the error message of validating uint32#10193

Merged
chia7712 merged 3 commits intoapache:trunkfrom
chia7712:MINOR-10193
Mar 2, 2021
Merged

MINOR: correct the error message of validating uint32#10193
chia7712 merged 3 commits intoapache:trunkfrom
chia7712:MINOR-10193

Conversation

@chia7712
Copy link
Copy Markdown
Member

as title

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

return (Long) item;
else
throw new SchemaException(item + " is not a Long.");
throw new SchemaException(item + " is not an unsigned integer.");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This would be inconsistent with the other messages, which refer to Java types. It's a condition on the Java type, so including it in the message is reasonable imho.

Suggested change
throw new SchemaException(item + " is not an unsigned integer.");
throw new SchemaException(item + " is not a Long (encoding an unsigned integer).");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Though now I look at the message for UINT16 I see it would be consistent with that. Still I think because there are two types involved here, the Java type and the network type, including both is clearest.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

make sense. will copy that.

Copy link
Copy Markdown
Member

@dajac dajac left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@chia7712 chia7712 merged commit 652cca3 into apache:trunk Mar 2, 2021
return (Long) item;
else
throw new SchemaException(item + " is not a Long.");
throw new SchemaException(item + " is not an a Long (encoding an unsigned integer).");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is a typo here, "an a".

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, I think it reads a bit weird. Not clear that "encoding an unsigned integer" in brackets means when reading the message.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is a typo here, "an a".

My bad :(

Also, I think it reads a bit weird. Not clear that "encoding an unsigned integer" in brackets means when reading the message.

I will update it in #10248 @ijuma thanks for your reviews

ijuma added a commit to ijuma/kafka that referenced this pull request Mar 2, 2021
* apache-github/trunk: (37 commits)
  KAFKA-10357: Extract setup of changelog from Streams partition assignor (apache#10163)
  KAFKA-10251: increase timeout for consuming records (apache#10228)
  KAFKA-12394; Return `TOPIC_AUTHORIZATION_FAILED` in delete topic response if no describe permission (apache#10223)
  MINOR: Disable transactional/idempotent system tests for Raft quorums (apache#10224)
  KAFKA-10766: Unit test cases for RocksDBRangeIterator (apache#9717)
  KAFKA-12289: Adding test cases for prefix scan in InMemoryKeyValueStore (apache#10052)
  KAFKA-12268: Implement task idling semantics via currentLag API (apache#10137)
  MINOR: Time and log producer state recovery phases (apache#10241)
  MINOR: correct the error message of validating uint32 (apache#10193)
  MINOR: Format the revoking active log output in `StreamsPartitionAssignor` (apache#10242)
  KAFKA-12323 Follow-up: Refactor the unit test a bit (apache#10205)
  MINOR: Remove stack trace of the lock exception in a debug log4j (apache#10231)
  MINOR: Word count should account for extra whitespaces between words (apache#10229)
  MINOR; Small refactor in `GroupMetadata` (apache#10236)
  KAFKA-10340: Proactively close producer when cancelling source tasks (apache#10016)
  KAFKA-12329; kafka-reassign-partitions command should give a better error message when a topic does not exist (apache#10141)
  KAFKA-12254: Ensure MM2 creates topics with source topic configs (apache#10217)
  MINOR: fix kafka-metadata-shell.sh (apache#10226)
  KAFKA-12374: Add missing config sasl.mechanism.controller.protocol (apache#10199)
  KAFKA-10101: Fix edge cases in Log.recoverLog and LogManager.loadLogs (apache#8812)
  ...
@chia7712 chia7712 deleted the MINOR-10193 branch March 25, 2024 15:21
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.

4 participants