default_discovery_dir should use YT username #22
Conversation
| def default_user(): | ||
| return os.getenv("YT_USER") or getpass.getuser() | ||
| def default_user(client=None): | ||
| return os.getenv("YT_USER") or (get_user_name(client=client) if client else None) or getpass.getuser() |
There was a problem hiding this comment.
There is a standard convention in almost all yt wrapper functions that they accept client as a last keyword argument, and client may be None, which means that the "default" client should be taken. In other words, I would expect that the "if client else None" part is not necessary;
Moreover, I would remove getpass.getuser() fallback, because there is no way that it can be useful. If the YT client is not configured, nothing would work at all, so get_user_name must succeed, and getuser is redundant.
| return os.getenv("SPARK_YT_DISCOVERY_DIR") or YPath("//home").join(os.getenv("USER")).join("spark-tmp") | ||
| def default_discovery_dir(client=None): | ||
| return os.getenv("SPARK_YT_DISCOVERY_DIR") \ | ||
| or YPath("//home").join(default_user(client=client)).join("spark-tmp") |
There was a problem hiding this comment.
Let's change this to //tmp? None of the known YT clusters allows having top-level home user dirs.
| @@ -218,8 +218,8 @@ def scala_buffer_to_list(buffer): | |||
| return [buffer.apply(i) for i in range(buffer.length())] | |||
|
|
|||
|
|
|||
| def default_user(): | |||
| return os.getenv("YT_USER") or getpass.getuser() | |||
There was a problem hiding this comment.
import getpass on the line 2 is no longer needed, so it also should be removed
|
@alextokarew has imported your pull request. If you are a Yandex employee, you can view this diff. |
|
✅ This pull request is being closed because it has been successfully merged into our internal monorepository. |
#20