SPARK-1557 Set permissions on event log files/directories #538
Conversation
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14447/ |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
| @@ -60,12 +62,12 @@ private[spark] class FileLogger( | |||
|
|
|||
| private var writer: Option[PrintWriter] = None | |||
|
|
|||
| createLogDir() | |||
| createLogDir(dirPermissions) | |||
There was a problem hiding this comment.
no need to pass this in. It's already accessible
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
|
Merged build triggered. |
|
Merged build started. |
| @@ -51,10 +52,12 @@ private[spark] class EventLoggingListener( | |||
| private val logBaseDir = conf.get("spark.eventLog.dir", "/tmp/spark-events").stripSuffix("/") | |||
| private val name = appName.replaceAll("[ :/]", "-").toLowerCase + "-" + System.currentTimeMillis | |||
| val logDir = logBaseDir + "/" + name | |||
| val LOG_FILE_PERMISSIONS: FsPermission = | |||
| FsPermission.createImmutable(Integer.parseInt("770", 8).toShort: Short) | |||
There was a problem hiding this comment.
No need to specify : Short; it's pretty clear what it is with toShort
There was a problem hiding this comment.
Also I think it's better if we move this to object EventLoggingListener, alongside with other scary all caps variables.
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
| if (dirPermissions.isDefined) { | ||
| val fsStatus = fileSystem.getFileStatus(path) | ||
| if (fsStatus.getPermission().toShort() != dirPermissions.get.toShort()) { | ||
| fileSystem.setPermission(path, dirPermissions.get); |
There was a problem hiding this comment.
nit: semicolon, also in the line above, do toShort without parentheses
|
Thanks again @tgraves. Pending the small changes this LGTM. |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
This adds minimal setting of event log directory/files permissions. To have a secure environment the user must manually create the top level event log directory and set permissions up. We can add logic to do that automatically later if we want. Author: Thomas Graves <[email protected]> Closes #538 from tgravescs/SPARK-1557 and squashes the following commits: e471d8e [Thomas Graves] rework d8b6620 [Thomas Graves] update use of octal 3ca9b79 [Thomas Graves] Updated based on comments 5a09709 [Thomas Graves] add in missing import 3150ed6 [Thomas Graves] SPARK-1557 Set permissions on event log files/directories (cherry picked from commit 8db0f7e) Signed-off-by: Thomas Graves <[email protected]>
This adds minimal setting of event log directory/files permissions. To have a secure environment the user must manually create the top level event log directory and set permissions up. We can add logic to do that automatically later if we want. Author: Thomas Graves <[email protected]> Closes apache#538 from tgravescs/SPARK-1557 and squashes the following commits: e471d8e [Thomas Graves] rework d8b6620 [Thomas Graves] update use of octal 3ca9b79 [Thomas Graves] Updated based on comments 5a09709 [Thomas Graves] add in missing import 3150ed6 [Thomas Graves] SPARK-1557 Set permissions on event log files/directories
…e better in anonymous uid scenarios
…ons (apache#538) ## Upstream SPARK-XXXXX ticket and PR link (if not applicable, explain) github: apache#24411 jira: https://issues.apache.org/jira/browse/SPARK-27514 ## What changes were proposed in this pull request? A previous change moved the removal of empty window expressions to the RemoveNoopOperations rule, which comes after the CollapseWindow rule. Therefore, by the time we get to CollapseWindow, we aren't guaranteed that empty windows have been removed. This change checks that the window expressions are not empty, and only collapses the windows if both windows are non-empty. A lengthier description and repro steps here: https://issues.apache.org/jira/browse/SPARK-27514 ## How was this patch tested? A unit test, plus I reran the breaking case mentioned in the Jira ticket.
* [SPARK-27267][CORE] Update snappy to avoid error when decompressing empty serialized data (apache#531) * [SPARK-27514][SQL] Skip collapsing windows with empty window expressions (apache#538) * Bump hadoop to 2.9.2-palantir.5 (apache#537)
1. The OpenStack official repo has been moved to `opendev.org`, we should also follow it. 2. openstack-infra has been migrated to openstack repo. Closes: theopenlab/openlab#284
This adds minimal setting of event log directory/files permissions. To have a secure environment the user must manually create the top level event log directory and set permissions up. We can add logic to do that automatically later if we want.