Skip to content

HDDS-4936. Change ozone groupId from org.apache.hadoop to org.apache.ozone #2018

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.

Sign up for GitHub

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

Jump to bottom
Merged
ChenSammi merged 22 commits into from
May 31, 2021
Merged

HDDS-4936. Change ozone groupId from org.apache.hadoop to org.apache.ozone #2018

ChenSammi merged 22 commits into from
May 31, 2021

Conversation

@ChenSammi
Copy link
Contributor

@ChenSammi ChenSammi commented Mar 10, 2021

@amaliujia
Copy link
Contributor

amaliujia commented Mar 10, 2021

I left a question in the JIRA for more context.

@ayushtkn
Copy link
Member

ayushtkn commented Mar 11, 2021

@ChenSammi is this change going to break any older application codes which have any of the ozone modules as dependency due to this?

@amaliujia
Copy link
Contributor

amaliujia commented Mar 11, 2021

LGTM

@ayushtkn I am thinking this change will only impact future releases. When users bump up versions, maybe change the group is ok?

@ChenSammi
Copy link
Contributor Author

ChenSammi commented Mar 11, 2021 •
edited
Loading

Current applications which depend on ozone modules, if they don't upgrade ozone module version, they need to do nothing.
If they want to upgrade ozone module version, say from 1.0.0 to 1.1.0, they have to change ozone module groupId from "org.apache.org" to "org.apache.ozone" at the same time, otherwise there will be maven module not found error.

@ayushtkn do you think it will be a big problem for current user?

@ayushtkn
Copy link
Member

ayushtkn commented Mar 11, 2021

Not sure, shouldn’t be a big problem, may be during release an explicit mention of this would be good or somewhere in the Release notes.
If you folks are convinced,Nothing blocking from my side.

@ChenSammi ChenSammi requested review from arp7 and elek March 11, 2021 09:47
elek
elek reviewed Mar 11, 2021
View reviewed changes
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 @ChenSammi, it looks good to me.

Do we plan to remove tha hadoop- prefixes from the artifactIds, too? Or in a separated patch?

@arp7
Copy link
Contributor

arp7 commented Mar 11, 2021

Hi @ChenSammi, can you please keep this PR open for a few days so more people can take a look and call out any potential incompatibilities?

@amaliujia
Copy link
Contributor

amaliujia commented Mar 11, 2021

Agree with @arp7. Maybe even better send a FYI email to dev@ as this might have big impact.

@ChenSammi
Copy link
Contributor Author

ChenSammi commented Mar 12, 2021

Thanks the patch @ChenSammi, it looks good to me.

Do we plan to remove tha hadoop- prefixes from the artifactIds, too? Or in a separated patch?

@elek , do you mean change "hadoop-main-ozone" to "ozone", change "hadoop-hdds" to "ozone-hdds", something like these?
I guess it requires changing the source code directory name as long as the artifactId, which will have obvious impact on current developers. Ideally, we should replace existing "hadoop" keywords as many as possible. But I 'm not sure how big the impact is by change the artifactId and directory name. Maybe I can send out an email to the dev list to collect more feedback

as @amaliujia has suggested.

@ChenSammi
Copy link
Contributor Author

ChenSammi commented Mar 12, 2021

Hi @ChenSammi, can you please keep this PR open for a few days so more people can take a look and call out any potential incompatibilities?

Sure.

@elek
Copy link
Member

elek commented Mar 12, 2021

do you mean change "hadoop-main-ozone" to "ozone", change "hadoop-hdds" to "ozone-hdds", something like these?

Yes, exactly.

Directory names and artifact names are different. Directory names are referenced only in the <modules> section of the parent pom files. ArtifactIds are used in other places together with groupId. artifactId can be changed without modifying the directory structure (but some direct reference to the final hadoop-ozone-filesystem-hadoop2-1.1.0-SNAPSHOT.jar may require update)

While directory rename affects only the developers (therefore I am fine with delay it) the artifactId should be changed in downstream projects. As we cause some minor inconveniences with the groupId change I would prefer to change both at the same time to cause inconvenience only once. (Can be done either in this patch or in another patch which follows this closely.)

@adoroszlai
Copy link
Contributor

adoroszlai commented Mar 12, 2021

As we cause some minor inconveniences with the groupId change I would prefer to change both at the same time to cause inconvenience only once.

Totally agree.

@ChenSammi
Copy link
Contributor Author

ChenSammi commented Mar 24, 2021 •
edited
Loading

Uploaded a new commit to remove Keyword “Hadoop” from the artificatID.

Hi @mukul1987 , the patch is ready for test.

@ChenSammi ChenSammi requested a review from elek March 24, 2021 07:20
@mukul1987 mukul1987 force-pushed the master branch 2 times, most recently from 79a9d39 to 520ba00 Compare March 25, 2021 16:05
@adoroszlai
Copy link
Contributor

adoroszlai commented Mar 26, 2021

@ChenSammi I had fixed several remaining issues in your branch:

Why did you overwrite those?

@ChenSammi
Copy link
Contributor Author

ChenSammi commented Mar 31, 2021 •
edited
Loading

@ChenSammi I had fixed several remaining issues in your branch:

* [98b6a07](https://github.com/apache/ozone/commit/98b6a07c2b5f4af2b003f68f2b4e5d2f6a27d37c) Fix IDEA run configs

* [57157df](https://github.com/apache/ozone/commit/57157df34a0cd02bf14d5f66ecc8638b8902ad29) Fix ozone-manager artifact name

* [2a24161](https://github.com/apache/ozone/commit/2a24161262cc12a4712225bb5bb03e4848aa3a38) Fix module names

* [715a956](https://github.com/apache/ozone/commit/715a95664e9f9efd510a1545d724915abc6cc1fb) Fix GH workflow

* [0f7bf9f](https://github.com/apache/ozone/commit/0f7bf9f721ce14416245153ebb23b055c773073f) Update module names in coverage.sh

* [81334ed](https://github.com/apache/ozone/commit/81334ed914d7facc6b21c4cceaff597b86228f2b) Fix checkstyle

Why did you overwrite those?

@adoroszlai , sorry, I didn't notice it's your fixes. I just thought it's part of the impact of previous master branch force push.

Would you please resubmit your comments again?

I saw you have changed some module names. It seems module name is related with module directory name. Since we don't change the module directory name in this patch, I'm not sure if new module names are applied.

@elek
Copy link
Member

elek commented Mar 31, 2021

I fetched the original commits from @adoroszlai (git fetch origin 98b6a07c2b5f4af2b003f68f2b4e5d2f6a27d37c) and merged it back to the latest branch from @ChenSammi and finally pushed it (git push [email protected]:ChenSammi/ozone.git HEAD:HDDS-4936)

Now it should contain all the fixes.

@ChenSammi
Copy link
Contributor Author

ChenSammi commented Apr 1, 2021 •
edited
Loading

I fetched the original commits from @adoroszlai (git fetch origin 98b6a07c2b5f4af2b003f68f2b4e5d2f6a27d37c) and merged it back to the latest branch from @ChenSammi and finally pushed it (git push [email protected]:ChenSammi/ozone.git HEAD:HDDS-4936)

Now it should contain all the fixes.

Thanks @elek for the help. I tried to rebase this branch with master to resolve the conflicting files, but met a lot of unexpected conflicts. So I manually applied @adoroszlai 's patche one by one. Here is the new MR #2104.

Let's continue the discussion in #2104.

@elek elek mentioned this pull request Apr 1, 2021
Closed
@ChenSammi ChenSammi closed this Apr 21, 2021
@ChenSammi ChenSammi reopened this Apr 21, 2021
@ChenSammi
Copy link
Contributor Author

ChenSammi commented May 28, 2021 •
edited
Loading

Hey @mukul1987 ， can we merge this PR now？

@mukul1987
Copy link
Contributor

mukul1987 commented May 28, 2021

@ChenSammi , lets merge the change then.

@ChenSammi
Copy link
Contributor Author

ChenSammi commented May 28, 2021

@ChenSammi , lets merge the change then.

Thanks @mukul1987 , would you help to approve the merge?

mukul1987
mukul1987 approved these changes May 28, 2021
View reviewed changes
Copy link
Contributor

@mukul1987 mukul1987 left a comment

Choose a reason for hiding this comment

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

LGTM

@ChenSammi ChenSammi merged commit fa4ddbc into apache:master May 31, 2021
@ChenSammi
Copy link
Contributor Author

ChenSammi commented May 31, 2021

Thanks @adoroszlai @elek @mukul1987 for the help.

errose28 added a commit to errose28/ozone that referenced this pull request Jun 1, 2021
@errose28
Merge remote-tracking branch 'upstream/master' into HDDS-3698-nonroll…
1d18f47 
…ing-upgrade-master-merge

* upstream/master: (76 commits)
  HDDS-5280. Make XceiverClientManager creation when necessary in ContainerOperationClient (apache#2289)
  HDDS-5272. Make ozonefs.robot execution repeatable (apache#2280)
  HDDS-5123. Use the pre-created apache/ozone-testkrb5 image during secure acceptance tests (apache#2165)
  HDDS-4993. Add guardrail for reserved buffer size when DN reads a chunk (apache#2058)
  HDDS-4936. Change ozone groupId from org.apache.hadoop to org.apache.ozone (apache#2018)
  HDDS-4043. allow deletion from Trash directory without -skipTrash option (apache#2110)
  HDDS-4927. Determine over and under utilized datanodes in Container Balancer. (apache#2230)
  HDDS-5273. Handle unsecure cluster convert to secure cluster for SCM. (apache#2281)
  HDDS-5158. Add documentation for SCM HA Security. (apache#2205)
  HDDS-5275. Datanode Report Publisher publishes one extra report after DN shutdown (apache#2283)
  HDDS-5241. SCM UI should have leader/follower and Primordial SCM information (apache#2260)
  HDDS-5219. Limit number of bad volumes by dfs.datanode.failed.volumes.tolerated. (apache#2243)
  HDDS-5252. PipelinePlacementPolicy filter out datanodes with not enough space. (apache#2271)
  HDDS-5191. Increase default pvc storage size (apache#2219)
  HDDS-5073. Use ReplicationConfig on client side  (apache#2136)
  HDDS-5250. Build integration tests with Maven cache (apache#2269)
  HDDS-5236. Require block token for more operations (apache#2254)
  HDDS-5266 Misspelt words in S3MultipartUploadCommitPartRequest.java line 202 (apache#2279)
  HDDS-5249. Race Condition between Full and Incremental Container Reports (apache#2268)
  HDDS-5142. Make generic streaming client/service for container re-replication, data read, scm/om snapshot download (apache#2256)
  ...

Conflicts:
	hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java
	hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java
	hadoop-hdds/interface-admin/src/main/proto/ScmAdminProtocol.proto
	hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
	hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java
	hadoop-ozone/dist/src/main/compose/testlib.sh
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestStorageContainerManager.java
	hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
	hadoop-ozone/ozone-manager/pom.xml
	hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
	hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
	hadoop-ozone/s3gateway/pom.xml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mukul1987 mukul1987 mukul1987 approved these changes

@arp7 arp7 Awaiting requested review from arp7

@elek elek Awaiting requested review from elek

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@ChenSammi @amaliujia @ayushtkn @arp7 @elek @adoroszlai @mukul1987