Skip to content

Conversation

@symious
Copy link
Contributor

@symious symious commented Mar 28, 2021

What changes were proposed in this pull request?

  • Use default config values to initialize configurations

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-5035

How was this patch tested?

The user's assembled fat jar works fines now.

@symious
Copy link
Contributor Author

symious commented Mar 30, 2021

@elek @adoroszlai Could you help to review this PR?

Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

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

Thanks the patch @symious

It looks good to me. I took me some time to test it, I think a simple unit test would help a lot to demonstrate the fix

Is there any chance to add a simple unit test (at least for a few types)?

As far as I see TestConfigurationReflectionUtil is parametrized (which makes it hard to add anything) but worst case we can add a new test class.

@symious
Copy link
Contributor Author

symious commented Apr 1, 2021

@elek Thanks for the review, have added a unit test to simulate this case. Please have a check.

Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

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

Thanks a lot the update @symious

LGTM.

To be honest, at first, it was not clear for me why do we need the file rename part. But I learned from the unit test, so it was definitely a help.

From now conf.getObject doesn't require any generated xml files. The more we use the typesafe annotations we are closer to remove the generated files...

@elek elek merged commit 843d39e into apache:master Apr 6, 2021
errose28 added a commit to errose28/ozone that referenced this pull request Apr 6, 2021
* master:
  HDDS-5066. Use fixed vesion from pnpm to build recon (apache#2115)
  HDDS-5035. Use default config values to solve generated config file conflict (apache#2087)
  HDDS-5032. DN stopped to load containers on volume after a container load exception. (apache#2109)
  HDDS-4504. Datanode deletion config should be based on number of blocks (apache#1885)
@symious
Copy link
Contributor Author

symious commented Apr 7, 2021

@elek Thanks for the review.
Sorry for not adding the unit test in the first place, it took some time to find a way to simulate the scenario for the unit test.

errose28 added a commit to errose28/ozone that referenced this pull request Apr 7, 2021
* HDDS-3698-nonrolling-upgrade: (144 commits)
  fix project name in NOTICE.txt (apache#2112)
  HDDS-5066. Use fixed vesion from pnpm to build recon (apache#2115)
  HDDS-5014. Add non-rolling upgrade design docs.
  HDDS-5035. Use default config values to solve generated config file conflict (apache#2087)
  HDDS-5032. DN stopped to load containers on volume after a container load exception. (apache#2109)
  HDDS-4504. Datanode deletion config should be based on number of blocks (apache#1885)
  Fix ozone-ha acceptance test.
  HDDS-5058. Make getScmInfo retry for a duration.
  HDDS-4506. Support query parameter based v4 auth in S3g (apache#1628)
  HDDS-4553. ChunkInputStream should release buffer as soon as last byte in the buffer is read (apache#2062)
  HDDS-5022. SCM get roles command should provide Ratis Leader/Follower… (apache#2098)
  HDDS-5033. SCM may not be able to know full port list of Datanode after Datanode is started. (apache#2090)
  HDDS-3752. Fix o3fs list bucket contents issue when without tailing "/" (apache#2088)
  HDDS-4901. Remove OmOzoneAclMap from OmVolumeArgs to avoid OzoneAcl conversions (apache#1992)
  HDDS-4987. Import container should not delete container contents if container already exists (apache#2077)
  Checkstyle fix.
  Intialize DN layout version before security init.
  HDDS-4915. [SCM HA Security] Integrate CertClient. (apache#2000)
  HDDS-5049. Add timeout support for ratis requests in SCM HA. (apache#2099)
  trigger new CI check
  ...
@elek
Copy link
Member

elek commented Apr 7, 2021

Sorry for not adding the unit test in the first place, it took some time to find a way to simulate the scenario for the unit test.

Sure, no problem. Now I understand that it was a tricky request, but earlier I thought it's a simpler problem ;-)

errose28 added a commit to errose28/ozone that referenced this pull request Apr 7, 2021
* HDDS-3698-nonrolling-upgrade:
  fix project name in NOTICE.txt (apache#2112)
  HDDS-5066. Use fixed vesion from pnpm to build recon (apache#2115)
  HDDS-5014. Add non-rolling upgrade design docs.
  HDDS-5035. Use default config values to solve generated config file conflict (apache#2087)
  HDDS-5032. DN stopped to load containers on volume after a container load exception. (apache#2109)
  HDDS-4504. Datanode deletion config should be based on number of blocks (apache#1885)
errose28 added a commit to errose28/ozone that referenced this pull request Apr 9, 2021
* HDDS-3698-nonrolling-upgrade: (150 commits)
  HDDS-5056. Avoid false positiver error messages during pipeline creations (apache#2105)
  HDDS-5027. [SCM HA Security] Handle leader changes during bootstrap. (apache#2113)
  HDDS-5032. Fix findbugs (apache#2120)
  HDDS-5062. Add a config to bypass clusterId validation for bootstrapping SCM. (apache#2114)
  HDDS-5011. Introduce Java based ReplicationConfig implementation (apache#2089)
  HDDS-4925. Introduce ContainerBalancer in SCM with start/stop capabilities. (apache#2097)
  fix project name in NOTICE.txt (apache#2112)
  HDDS-5066. Use fixed vesion from pnpm to build recon (apache#2115)
  HDDS-5014. Add non-rolling upgrade design docs.
  HDDS-5035. Use default config values to solve generated config file conflict (apache#2087)
  HDDS-5032. DN stopped to load containers on volume after a container load exception. (apache#2109)
  HDDS-4504. Datanode deletion config should be based on number of blocks (apache#1885)
  Fix ozone-ha acceptance test.
  HDDS-5058. Make getScmInfo retry for a duration.
  HDDS-4506. Support query parameter based v4 auth in S3g (apache#1628)
  HDDS-4553. ChunkInputStream should release buffer as soon as last byte in the buffer is read (apache#2062)
  HDDS-5022. SCM get roles command should provide Ratis Leader/Follower… (apache#2098)
  HDDS-5033. SCM may not be able to know full port list of Datanode after Datanode is started. (apache#2090)
  HDDS-3752. Fix o3fs list bucket contents issue when without tailing "/" (apache#2088)
  HDDS-4901. Remove OmOzoneAclMap from OmVolumeArgs to avoid OzoneAcl conversions (apache#1992)
  ...
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