[ALLUXIO-2393] Remove the Permission class. #4717

jsimsa · 2017-02-01T06:00:46Z

Addresses https://alluxio.atlassian.net/browse/ALLUXIO-2393

The motivation behind this PR is to decouple owner, group, and mode of path permissions as these attributes are often initialized through different means, which presents challenges for the Permission class API.

The main change in this PR is that the Permission class is removed. Its methods for initializing owner and group using login module and thrift client are moved to SecurityUtils and its methods for applying umask are moved to Mode.

The rest of the PR are mostly mechanical changes that replace uses of Permission with equivalent uses of owner, group, and mode.

AmplabJenkins · 2017-02-01T06:01:45Z

Merged build finished. Test FAILed.

AmplabJenkins · 2017-02-01T06:01:46Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/13246/
Test FAILed.

AmplabJenkins · 2017-02-01T06:07:40Z

Merged build finished. Test FAILed.

AmplabJenkins · 2017-02-01T06:07:40Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/13247/
Test FAILed.

AmplabJenkins · 2017-02-01T06:40:50Z

Merged build finished. Test FAILed.

AmplabJenkins · 2017-02-01T06:40:50Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/13248/

Failed Tests: 1

org.alluxio:alluxio-core-client: 1

alluxio.client.file.options.CreateFileOptionsTest.toThrift

Test FAILed.

AmplabJenkins · 2017-02-01T06:46:46Z

Merged build finished. Test FAILed.

AmplabJenkins · 2017-02-01T06:46:47Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/13249/

Failed Tests: 0

Test FAILed.

maobaolong · 2017-02-01T06:56:25Z

#4704 My Pr blocked by the failed test too.

jsimsa · 2017-02-01T15:50:26Z

Jenkins, test this please

jsimsa · 2017-02-01T16:14:11Z

core/server/src/test/java/alluxio/master/file/PermissionCheckTest.java

        inodes.add(inode);
      } else {
        Inode<?> inode = InodeDirectory.create(i + 1, i, (i + 1) + "",
-            CreateDirectoryOptions.defaults().setPermission(permissions.get(i)));
+            CreateDirectoryOptions.defaults().setOwner(owner).setGroup(group).setMode(mode));
+        inodes.add(inode);


this was a bug

AmplabJenkins · 2017-02-01T16:21:46Z

Merged build finished. Test FAILed.

AmplabJenkins · 2017-02-01T16:21:46Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/13257/
Test FAILed.

jsimsa · 2017-02-01T16:22:43Z

Jenkins, test this please

AmplabJenkins · 2017-02-01T16:25:41Z

Merged build finished. Test FAILed.

AmplabJenkins · 2017-02-01T16:25:42Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/13256/

Failed Tests: 2

org.alluxio:alluxio-core-client: 1

alluxio.client.file.options.CreateFileOptionsTest.toThrift

org.alluxio:alluxio-tests: 1

alluxio.master.MasterFaultToleranceIntegrationTest.workerReRegister

Test FAILed.

AmplabJenkins · 2017-02-01T16:46:23Z

Merged build finished. Test FAILed.

AmplabJenkins · 2017-02-01T16:46:23Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/13258/

Failed Tests: 1

org.alluxio:alluxio-core-client: 1

alluxio.client.file.options.CreateFileOptionsTest.toThrift

Test FAILed.

AmplabJenkins · 2017-02-01T16:56:29Z

Merged build finished. Test PASSed.

AmplabJenkins · 2017-02-01T16:56:29Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/13259/
Test PASSed.

alluxio-bot · 2017-02-01T16:56:30Z

Automated checks report:

Valid pull request title: PASS
Contains link to JIRA ticket: PASS
Commits associated with Github account: PASS

All checks passed!

apc999

thanks for the improve! It makes the code much cleaner now

[SMALLFIX] Follow up to #4717

ifcharming · 2017-02-06T19:43:13Z

LGTM, thanks for the refactoring!

jsimsa added 2 commits January 30, 2017 17:03

Removing the permission class.

ee9e32a

Fixing up test failures.

58e0b4c

jsimsa force-pushed the permission branch from 78a7f5c to 58e0b4c Compare February 1, 2017 06:01

jsimsa added 2 commits January 31, 2017 22:17

Fix checkstyle.

19b4149

Removing unused code.

d77bdd8

jsimsa requested review from apc999 and ifcharming February 1, 2017 15:54

jsimsa commented Feb 1, 2017

View reviewed changes

Mode cleanup.

eef457d

jsimsa requested a review from calvinjia February 1, 2017 16:22

Client-side option test improvements.

9215283

apc999 approved these changes Feb 2, 2017

View reviewed changes

jsimsa merged commit 93708ea into Alluxio:master Feb 3, 2017

jsimsa mentioned this pull request Feb 3, 2017

[SMALLFIX] Follow up to #4717 #4724

Merged

jsimsa added a commit that referenced this pull request Feb 3, 2017

Merge pull request #4724 from jsimsa/permission-mode

8fc68e5

[SMALLFIX] Follow up to #4717

jsimsa deleted the permission branch March 15, 2017 17:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ALLUXIO-2393] Remove the Permission class. #4717

[ALLUXIO-2393] Remove the Permission class. #4717

jsimsa commented Feb 1, 2017 •

edited

Loading

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

maobaolong commented Feb 1, 2017

jsimsa commented Feb 1, 2017

jsimsa Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

jsimsa commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

alluxio-bot commented Feb 1, 2017

apc999 left a comment

ifcharming commented Feb 6, 2017

[ALLUXIO-2393] Remove the Permission class. #4717

[ALLUXIO-2393] Remove the Permission class. #4717

Conversation

jsimsa commented Feb 1, 2017 • edited Loading

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

Failed Tests: 1

org.alluxio:alluxio-core-client: 1

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

Failed Tests: 0

maobaolong commented Feb 1, 2017

jsimsa commented Feb 1, 2017

jsimsa Feb 1, 2017

Choose a reason for hiding this comment

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

jsimsa commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

Failed Tests: 2

org.alluxio:alluxio-core-client: 1

org.alluxio:alluxio-tests: 1

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

Failed Tests: 1

org.alluxio:alluxio-core-client: 1

AmplabJenkins commented Feb 1, 2017

AmplabJenkins commented Feb 1, 2017

alluxio-bot commented Feb 1, 2017

apc999 left a comment

Choose a reason for hiding this comment

ifcharming commented Feb 6, 2017

jsimsa commented Feb 1, 2017 •

edited

Loading