You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/08/18 09:39:05 UTC
[GitHub] [incubator-pinot] laxmanchekka opened a new pull request #5890: Fix gcsFS copyToLocal to precreate local destination directory
laxmanchekka opened a new pull request #5890:
URL: https://github.com/apache/incubator-pinot/pull/5890
## Description
Fix the error while downloading segment from GCS via controller.
https://github.com/apache/incubator-pinot/issues/5884
## Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
No
Does this PR fix a zero-downtime upgrade introduced earlier?
No
Does this PR otherwise need attention when creating release notes? Things to consider:
NA
## Release Notes
NA
## Documentation
NA
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 merged pull request #5890: Fix gcsFS copyToLocal to precreate local destination directory
Posted by GitBox <gi...@apache.org>.
fx19880617 merged pull request #5890:
URL: https://github.com/apache/incubator-pinot/pull/5890
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on pull request #5890: Fix gcsFS copyToLocal to precreate local destination directory
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on pull request #5890:
URL: https://github.com/apache/incubator-pinot/pull/5890#issuecomment-675684093
Thanks for fixing this!
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] laxmanchekka commented on a change in pull request #5890: Fix gcsFS copyToLocal to precreate local destination directory
Posted by GitBox <gi...@apache.org>.
laxmanchekka commented on a change in pull request #5890:
URL: https://github.com/apache/incubator-pinot/pull/5890#discussion_r472395046
##########
File path: pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsPinotFS.java
##########
@@ -325,6 +326,7 @@ public long length(URI fileUri) throws IOException {
@Override
public void copyToLocalFile(URI srcUri, File dstFile) throws Exception {
LOGGER.info("Copy {} to local {}", srcUri, dstFile.getAbsolutePath());
+ FileUtils.forceMkdir(dstFile.getParentFile());
Review comment:
No. This util is idempotent. Doesn't throw any exception even if directory exists.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg commented on a change in pull request #5890: Fix gcsFS copyToLocal to precreate local destination directory
Posted by GitBox <gi...@apache.org>.
kishoreg commented on a change in pull request #5890:
URL: https://github.com/apache/incubator-pinot/pull/5890#discussion_r472362632
##########
File path: pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsPinotFS.java
##########
@@ -325,6 +326,7 @@ public long length(URI fileUri) throws IOException {
@Override
public void copyToLocalFile(URI srcUri, File dstFile) throws Exception {
LOGGER.info("Copy {} to local {}", srcUri, dstFile.getAbsolutePath());
+ FileUtils.forceMkdir(dstFile.getParentFile());
Review comment:
should we check the existence before creating? will forceMkdir throw an exception if the directory exists?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org