SbloodyS commented on code in PR #10433: URL: https://github.com/apache/dolphinscheduler/pull/10433#discussion_r897984480
########## docs/docs/zh/architecture/configuration.md: ########## @@ -178,18 +178,20 @@ common.properties配置文件目前主要是配置hadoop/s3a相关的配置. |--|--|--| data.basedir.path|/tmp/dolphinscheduler|本地工作目录,用于存放临时文件 resource.storage.type|NONE|资源文件存储类型: HDFS,S3,NONE -resource.upload.path|/dolphinscheduler|资源文件存储路径 +resource.storage.upload.base.path|/dolphinscheduler|资源文件存储路径 +resource.aws.access.key.id| minioadmin| S3 access key +resource.aws.secret.access.key| minioadmin| S3 secret access key +resource.aws.region|us-east-1| S3 区域 +resource.aws.s3.bucket.name| dolphinscheduler-test| S3 存储桶名称 +resource.aws.s3.endpoint| http://minio:9000| s3 endpoint地址 +resource.hdfs.root.user|hdfs|如果存储类型为HDFS,需要配置拥有对应操作权限的用户 +resource.hdfs.fs.defaultFS|hdfs://mycluster:8020|请求地址如果resource.storage.type=S3,该值类似为: s3a://dolphinscheduler. 如果resource.storage.type=HDFS, 如果 hadoop 配置了 HA,需要复制core-site.xml 和 hdfs-site.xml 文件到conf目录 Review Comment: Same as english's doc. ########## dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/S3Utils.java: ########## @@ -264,25 +308,27 @@ private void deleteTenantCode(String tenantCode) { /** * xxx untest * upload local directory to S3 + * * @param tenantCode - * @param keyPrefix the name of directory + * @param keyPrefix the name of directory Review Comment: We should avoid unsessnary change. ########## docs/docs/en/architecture/configuration.md: ########## @@ -185,19 +185,21 @@ Currently, common.properties mainly configures Hadoop,s3a related configurations |Parameters | Default value| Description| |--|--|--| data.basedir.path|/tmp/dolphinscheduler| local directory used to store temp files -resource.storage.type|NONE| type of resource files: HDFS, S3, NONE -resource.upload.path|/dolphinscheduler| storage path of resource files +resource.view.suffixs| txt,log,sh,conf,cfg,py,java,sql,hql,xml,properties| file types supported by resource center +resource.storage.type| NONE| type of resource files: HDFS, S3, NONE +resource.storage.upload.base.path| /dolphinscheduler| storage path of resource files +resource.aws.access.key.id| minioadmin| access key id of S3 +resource.aws.secret.access.key| minioadmin| secret access key of S3 +resource.aws.region|us-east-1| region of S3 +resource.aws.s3.bucket.name| dolphinscheduler-test| bucket name of S3 +resource.aws.s3.endpoint| http://minio:9000| endpoint of S3 +resource.hdfs.root.user| hdfs| configure users with corresponding permissions if storage type is HDFS +resource.hdfs.fs.defaultFS| hdfs://mycluster:8020|If resource.storage.type=S3, then the request url would be similar to 's3a://dolphinscheduler'. Otherwise if resource.storage.type=HDFS and hadoop supports HA, copy core-site.xml and hdfs-site.xml into 'conf' directory Review Comment: We should add a space between each ```|``` to keep the format consistent. ########## dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/S3Utils.java: ########## @@ -221,33 +236,21 @@ public boolean upload(String tenantCode, String srcFile, String dstPath, boolean s3Client.putObject(BUCKET_NAME, dstPath, new File(srcFile)); return true; } catch (AmazonServiceException e) { - logger.error("upload failed,the bucketName is {},the dstPath is {}", BUCKET_NAME, tenantCode+ FOLDER_SEPARATOR +dstPath); + logger.error("upload failed,the bucketName is {},the dstPath is {}", BUCKET_NAME, tenantCode + FOLDER_SEPARATOR + dstPath); Review Comment: ```suggestion logger.error("upload failed,the bucketName is {},the ds Path is {}", BUCKET_NAME, tenantCode + FOLDER_SEPARATOR + dstPath); ``` ########## dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/S3Utils.java: ########## @@ -293,15 +339,28 @@ private void downloadDirectory(String tenantCode, String keyPrefix, String srcP } } - public void checkBucketNameIfNotPresent(String bucketName) { - if (!s3Client.doesBucketExistV2(bucketName)) { - logger.info("the current regionName is {}", s3Client.getRegionName()); - s3Client.createBucket(bucketName); + public void checkBucketNameExists(String bucketName) { + if (StringUtils.isBlank(bucketName)) { + Stopper.stop(); + throw new IllegalArgumentException("resource.aws.s3.bucket.name is blank"); } + + Bucket existsBucket = s3Client.listBuckets() + .stream() + .filter( + bucket -> bucket.getName().equals(bucketName) + ) + .findFirst() + .orElseThrow(() -> { + Stopper.stop(); Review Comment: Same as line 344. ########## dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/S3Utils.java: ########## @@ -293,15 +339,28 @@ private void downloadDirectory(String tenantCode, String keyPrefix, String srcP } } - public void checkBucketNameIfNotPresent(String bucketName) { - if (!s3Client.doesBucketExistV2(bucketName)) { - logger.info("the current regionName is {}", s3Client.getRegionName()); - s3Client.createBucket(bucketName); + public void checkBucketNameExists(String bucketName) { + if (StringUtils.isBlank(bucketName)) { + Stopper.stop(); Review Comment: I think we should not stop in here. ########## dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/S3Utils.java: ########## @@ -264,25 +308,27 @@ private void deleteTenantCode(String tenantCode) { /** * xxx untest * upload local directory to S3 + * * @param tenantCode - * @param keyPrefix the name of directory + * @param keyPrefix the name of directory * @param strPath */ private void uploadDirectory(String tenantCode, String keyPrefix, String strPath) { - s3Client.putObject(BUCKET_NAME, tenantCode+ FOLDER_SEPARATOR +keyPrefix, new File(strPath)); + s3Client.putObject(BUCKET_NAME, tenantCode + FOLDER_SEPARATOR + keyPrefix, new File(strPath)); } /** * xxx untest * download S3 Directory to local + * * @param tenantCode - * @param keyPrefix the name of directory + * @param keyPrefix the name of directory Review Comment: Unsessnary change. ########## dolphinscheduler-common/src/main/resources/common.properties: ########## @@ -18,11 +18,29 @@ # user data local directory path, please make sure the directory exists and have read write permissions data.basedir.path=/tmp/dolphinscheduler +# resource view suffixs +#resource.view.suffixs=txt,log,sh,bat,conf,cfg,py,java,sql,xml,hql,properties,json,yml,yaml,ini,js + # resource storage type: HDFS, S3, NONE resource.storage.type=NONE +# resource store on HDFS/S3 path, resource file will store to this base path, self configuration, please make sure the directory exists on hdfs and have read write permissions. "/dolphinscheduler" is recommended +resource.storage.upload.base.path=/dolphinscheduler + +# The AWS access key. if resource.storage.type=S3 or use EMR-Task, This configuration is required +resource.aws.access.key.id=minioadmin +# The AWS secret access key. if resource.storage.type=S3 or use EMR-Task, This configuration is required +resource.aws.secret.access.key=minioadmin +# The AWS Region to use. if resource.storage.type=S3 or use EMR-Task, This configuration is required +resource.aws.region=cn-north-1 +# The name of the bucket. You need to create them by yourself. Otherwise, the system cannot start. All buckets in Amazon S3 share a single namespace; ensure the bucket is given a unique name. +resource.aws.s3.bucket.name=dolphinscheduler-test Review Comment: I think we should use a formal name such as ```dolphinscheduler``` instead of test since this is not a test configuration file. ########## docs/docs/en/architecture/configuration.md: ########## @@ -185,19 +185,21 @@ Currently, common.properties mainly configures Hadoop,s3a related configurations |Parameters | Default value| Description| |--|--|--| data.basedir.path|/tmp/dolphinscheduler| local directory used to store temp files -resource.storage.type|NONE| type of resource files: HDFS, S3, NONE -resource.upload.path|/dolphinscheduler| storage path of resource files +resource.view.suffixs| txt,log,sh,conf,cfg,py,java,sql,hql,xml,properties| file types supported by resource center Review Comment: We have removed the suffix restriction from the content center in #9553. So please remove it. -- 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]
