Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GOBBLIN-1866] Preserve sticky bit across distcp copies #3726

Merged
merged 2 commits into from
Aug 2, 2023

Conversation

y242yang
Copy link
Contributor

@y242yang y242yang commented Jul 27, 2023

Dear Gobblin maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

Description

Gobblin distcp copies the file permissions but not the sticky bit. As the expectation is that the destination files have preserved permissions from the source, we want to maintain the sticky bit across copies.

Tests

-- Unit test to compare the input and output FsPermission with and without sticky bits with different permission modes
-- A workflow on HDFS where a directory is copied from one location to another.

Commits

☑︎ My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":

  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

Copy link
Contributor

@meethngala meethngala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks good and it will help us preserve sticky bit for directories which was missing earlier. Can we add or update unit tests for testing the preserve logic for sticky bit? You can take a look at this: FileAwareInputStreamDataWriterTest.testAddExecutePermission()

String[] expectPermissions = {"100", "100", "300", "500", "700", "700", "311", "350"};
String[] stickyBit = {"" ,"1"};
for (String bit : stickyBit) {
for (int index = 0; index < setPermissions.length; ++index) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how you're trying to minimize and reuse the code... :)!

@Will-Lo Will-Lo changed the title [gobblin] move sticky bit [GOBBLIN-1866] Preserve sticky bit across distcp copies Aug 2, 2023
@Will-Lo Will-Lo merged commit 2917b63 into apache:master Aug 2, 2023
6 checks passed
phet pushed a commit to phet/gobblin that referenced this pull request Aug 15, 2023
* [gobblin] move sticky bit

* [gobblin] add unit test to sticky bit

---------

Co-authored-by: Yiming Yang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants