[ https://issues.apache.org/jira/browse/HDDS-2378?focusedWorklogId=335779&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-335779 ]
ASF GitHub Bot logged work on HDDS-2378: ---------------------------------------- Author: ASF GitHub Bot Created on: 29/Oct/19 21:36 Start Date: 29/Oct/19 21:36 Worklog Time Spent: 10m Work Description: fapifta commented on pull request #103: HDDS-2378 - Change OZONE as string used in the code where OzoneConsts.OZONE is suitable URL: https://github.com/apache/hadoop-ozone/pull/103 ## What changes were proposed in this pull request? In #95 review it came up that the tests could use the OzoneConsts.OZONE constant instead of using the "ozone" string literal, this PR is aiming to address this issue globally in the whole codebase. As it is commented in the JIRA the changes were made in this PR where suitable in the following way: 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) ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-2378 ## How was this patch tested? There are no change in the code logic, mostly tests were changed, the expectation is that all the JUnit tests should run as they ran before. I have ran most of the changed tests locally, they seemed ok. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking ------------------- Worklog Id: (was: 335779) Remaining Estimate: 0h Time Spent: 10m > 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 > Labels: pull-request-available > Time Spent: 10m > Remaining Estimate: 0h > > 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