[ 
https://issues.apache.org/jira/browse/HDDS-2378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16962479#comment-16962479
 ] 

Istvan Fajth commented on HDDS-2378:
------------------------------------

After going through the code, the following came out of this:
There are some places where we use the string literal "Ozone" which I would not 
change, the places are:
- RocksDBStore, and RDBStore, where it is used to specify the name of jmx keys.
- ServerUtils uses it in a function parameter passed to a log message that 
seems fine.
- StorageContainerManager, and OzoneManager uses the full capital in 
InterfaceAudience annotation, that is fine.
- CommonHeadersContainerResponseFilter is using this in headers I would not 
change the protocol

In TestAuthorizationHeaderV4 we use "ozone" that is ok, as we provide a string 
to parse, and check elements provided, in this case I would not change to a 
constant makes it harder to read.

CreateSubcommand I am unsure, the default is specified with full capital I am 
not brave enough to change it to the lowercase version, though my instinct 
tells that it would be most likely ok.


Also while going through the S3 related tests, that were coming up with the 
query, I changed a few other literals to suitable constants, like:
"OWNER" -> OzoneConsts.OWNER (makes it lowercase)
"VOLUME" -> OzoneConsts.VOLUME (makes it lowercase)
"hdfs" -> OzoneConsts.OZONE_SIMPLE_HDFS_USER
"xxBucket" -> OzoneConsts.BUCKET
"s3Bucket" -> OzoneConsts.S3_BUCKET
"key1" -> OzoneConsts.KEY (in cases where only 1 key is used)

Preparing the PR shortly.

> Change "OZONE" as string used in the code where OzoneConsts.OZONE is suitable
> -----------------------------------------------------------------------------
>
>                 Key: HDDS-2378
>                 URL: https://issues.apache.org/jira/browse/HDDS-2378
>             Project: Hadoop Distributed Data Store
>          Issue Type: Improvement
>          Components: test
>    Affects Versions: 0.4.1
>            Reporter: Istvan Fajth
>            Assignee: Istvan Fajth
>            Priority: Major
>
> Based on a review I have done a quick check, and there are quite a few places 
> where we have hardcoded "ozone" as String literal or a capital version of it 
> into the code.
> Let's check then one by one, and where it is possible replace it with 
> OzoneConsts.OZONE, or if the lower case version is not acceptable at all 
> places, then create an other constant with the uppercase version and use that.
> This is the search, and the results:
> {code:bash}
> find . -name *.java | while read FILE; do NUM=`grep -c -i "\"OZONE\"" $FILE`; 
> if [ $NUM -gt 0 ]; then echo $FILE; fi; done | sort | uniq
> ./hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStore.java
> ./hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStore.java
> ./hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
> ./hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDataYaml.java
> ./hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestBlockManagerImpl.java
> ./hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
> ./hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/ServerUtils.java
> ./hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/metrics/SCMContainerManagerMetrics.java
> ./hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementMetrics.java
> ./hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeMetrics.java
> ./hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/SCMPipelineMetrics.java
> ./hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMContainerMetrics.java
> ./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/block/TestBlockManager.java
> ./hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestCloseContainerEventHandler.java
> ./hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestSCMContainerManager.java
> ./hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java
> ./hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/CreateSubcommand.java
> ./hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/util/OzoneVersionInfo.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerStateManagerIntegration.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/container/metrics/TestSCMContainerManagerMetrics.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerOperations.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerStateMachineIdempotency.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestStorageContainerManager.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/Test2WayCommitInRatis.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestCommitWatcher.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestWatchForCommit.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/ozShell/TestS3Shell.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestAllocateContainer.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestContainerSmallFile.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestGetCommittedBlockLengthAndPutKey.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestXceiverClientManager.java
> ./hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestXceiverClientMetrics.java
> ./hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
> ./hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestS3BucketManager.java
> ./hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerDoubleBufferWithOMResponse.java
> ./hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMAllocateBlockRequest.java
> ./hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCommitRequest.java
> ./hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
> ./hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/bucket/TestS3BucketDeleteRequest.java
> ./hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/bucket/TestS3BucketDeleteResponse.java
> ./hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/CommonHeadersContainerResponseFilter.java
> ./hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestAbortMultipartUpload.java
> ./hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketDelete.java
> ./hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketHead.java
> ./hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestInitiateMultipartUpload.java
> ./hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestListParts.java
> ./hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestMultipartUploadComplete.java
> ./hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestMultipartUploadWithCopy.java
> ./hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java
> ./hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPartUpload.java
> ./hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestRootList.java
> ./hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/header/TestAuthorizationHeaderV4.java
> ./hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/genesis/BenchMarkContainerStateMap.java
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to