Skip to content

Conversation

@sky76093016
Copy link
Contributor

What changes were proposed in this pull request?

Avoid methods that generate NPE when calling unit().

What is the link to the Apache JIRA

https://issues.apache.org/jira/projects/HDDS/issues/HDDS-5558

How was this patch tested?

No need.

@swagle swagle requested a review from adoroszlai August 9, 2021 16:04

long raw = Long.parseLong(vStr);
long converted = unit.convert(raw, vUnit.unit());
long converted = unit.convert(raw, Objects.requireNonNull(vUnit).unit());
Copy link
Contributor

Choose a reason for hiding this comment

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

if vUnit can be null, it would be better to check its nullity right after line 52, and throw an exception there (which can carry a more meaningful error message).

Copy link
Contributor

Choose a reason for hiding this comment

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

@jojochuang this is addressed in latest patch. Can you please take another look?

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

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

let's change it.

@sky76093016
Copy link
Contributor Author

Thanks @jojochuang for reviewing this.I have solved it by throwing an exception.

@sky76093016 sky76093016 requested a review from jojochuang August 10, 2021 08:02
Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @sky76093016 for working on this.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @sky76093016 for updating the patch.

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

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

+1 i'll merge it now. thanks @adoroszlai

@jojochuang jojochuang merged commit 1c96a5b into apache:master Aug 11, 2021
errose28 added a commit to errose28/ozone that referenced this pull request Aug 12, 2021
* master:
  HDDS-5358. Incorrect cache entry invalidation causes intermittent failure in testGetS3SecretAndRevokeS3Secret (apache#2518)
  HDDS-5608. Fix wrong command in ugrade doc (apache#2524)
  HDDS-5000. Run CI checks selectively (apache#2479)
  HDDS-4929. Select target datanodes and containers to move for Container Balancer (apache#2441)
  HDDS-5283. getStorageSize cast to int can cause issue (apache#2303)
  HDDS-5449 Recon namespace summary 'du' information should return replicated size of a key (apache#2489)
  HDDS-5558. vUnit invocation unit() may produce NPE (apache#2513)
  HDDS-5531. For Link Buckets avoid showing metadata. (apache#2502)
  HDDS-5549. Add 1.1 to supported versions in security policy (apache#2519)
  HDDS-5555. remove pipeline manager v1 code (apache#2511)
  HDDS-5546.OM Service ID change causes OM startup failure. (apache#2512)
  HDDS-5360. DN failed to process all delete block commands in one heartbeat interval (apache#2420)
  HDDS-5021. dev-support Dockerfile is badly outdated (apache#2480)
errose28 added a commit to errose28/ozone that referenced this pull request Aug 12, 2021
* master:
  HDDS-5358. Incorrect cache entry invalidation causes intermittent failure in testGetS3SecretAndRevokeS3Secret (apache#2518)
  HDDS-5608. Fix wrong command in ugrade doc (apache#2524)
  HDDS-5000. Run CI checks selectively (apache#2479)
  HDDS-4929. Select target datanodes and containers to move for Container Balancer (apache#2441)
  HDDS-5283. getStorageSize cast to int can cause issue (apache#2303)
  HDDS-5449 Recon namespace summary 'du' information should return replicated size of a key (apache#2489)
  HDDS-5558. vUnit invocation unit() may produce NPE (apache#2513)
  HDDS-5531. For Link Buckets avoid showing metadata. (apache#2502)
  HDDS-5549. Add 1.1 to supported versions in security policy (apache#2519)
  HDDS-5555. remove pipeline manager v1 code (apache#2511)
  HDDS-5546.OM Service ID change causes OM startup failure. (apache#2512)
  HDDS-5360. DN failed to process all delete block commands in one heartbeat interval (apache#2420)
  HDDS-5021. dev-support Dockerfile is badly outdated (apache#2480)
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.

3 participants