GeorgeJahad commented on a change in pull request #2629:
URL: https://github.com/apache/ozone/pull/2629#discussion_r705909015
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -2682,27 +2682,8 @@ public void renameKey(OmKeyArgs args, String toKeyName)
throws IOException {
*/
@Override
public void deleteKey(OmKeyArgs args) throws IOException {
- Map<String, String> auditMap = args.toAuditMap();
- try {
- ResolvedBucket bucket = resolveBucketLink(args);
- args = bucket.update(args);
-
- if (isAclEnabled) {
- checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.DELETE,
- bucket.realVolume(), bucket.realBucket(), args.getKeyName());
- }
-
- metrics.incNumKeyDeletes();
- keyManager.deleteKey(args);
- AUDIT.logWriteSuccess(buildAuditMessageForSuccess(OMAction.DELETE_KEY,
- auditMap));
- metrics.decNumKeys();
- } catch (Exception ex) {
- metrics.incNumKeyDeleteFails();
- AUDIT.logWriteFailure(buildAuditMessageForFailure(OMAction.DELETE_KEY,
- auditMap, ex));
- throw ex;
- }
+ throw new UnsupportedOperationException("OzoneManager does not require " +
Review comment:
> Just a question, instead of doing this way, is there a way we can
remove these methods.
I haven't looked at that much yet, because I saw that deleteKeys(),
renameKeys(), setQuota() and revokeS3Secret() all use the
UnsupportedOperationException approach, so I assumed that is what I was
supposed to do. I just copied the code from deleteKeys()
> My question is coming from the context, as if any new write requests
> added to OzoneManagerProtocol, still we should add this
> UnsupportedOperationException?
I think so, unfortunately, until we redefine the OzoneManagerProtocol. And
as @cxorm says, that is likely to be a huge patch.
> If that is too much change, we can explore that in a new Jira. Lets
> get the work going in, as I see it is causing lot of confusion to OM
> developers
I think that is the right decision. I think there are two separate problems
1. removing the old unused methods, (and fixing the tests that use them).
2. fixing the OzoneManagerProtocol
As of right now, I think they can be handled in separate PR's. And I think
1 is much higher priority, because as you've said, the code is causing so much
confusion. What do you think, @bharatviswa504 ?
> And also in similar mannner are you also planning to delete
> KeyManagerImpl delete method also? (I think we can completely remove
> that method)
Yes, but I think the most efficient way to do it is to finish each class in
'org.apached.hadoop.ozone.om' one at a time.
In other words, finish removing all the methods from OzoneManager.java
first. And fix all the tests that that breaks. Then all the methods from the
KeyManager.java and fix the tests that those break, and so on. Does that sound
ok, @bharatviswa504 ?
> I ever used to the same throw-Exception manner and I think it doesn't
solve the problem fully.
So do you think it is ok to just continue on with the OM for now, and add
UnsupportedOperationException's for the rest of the methods, before moving on
to the other classes? Would that cause any problems @cxorm ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
