Github user ryan-williams commented on the pull request:
https://github.com/apache/spark/pull/2848#issuecomment-65834877
OK @JoshRosen I fixed and cleaned things up.
* the two overloaded `maybeMoveFile` signatures are more distinctly named
(`downloadStreamAndMove` and `copyFile`)
* each is called twice, and the former actually uses the latter, so I think
there are some overall code-reuse wins, in addition to the "don't move/copy if
the thing is already there" bug fix happening in one place.
I'm open to writing some tests but it's not that obvious how to do so in a
way that is meaningful / doesn't just duplicate the logic in the file itself.
The "fetch file" code path is not tested in the existing `UtilsSuite` afaict,
and it doesn't feel worth adding an integration test around, imho. lmk what
else you'd like.
Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
