HDFS-8631. WebHDFS : Support setQuota #1253
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sunchao
changed the title
goiri
reviewed
Aug 8, 2019
| */ | ||
| public void setQuota(Path src, final long namespaceQuota, | ||
| final long storagespaceQuota) throws IOException { | ||
| throw new UnsupportedOperationException(getClass().getCanonicalName() + |
There was a problem hiding this comment.
This is a pattern used in many places.
I think we should have a static method to do this.
I think the easiest way to get the function name is to get the stack trace.
There was a problem hiding this comment.
Agree. Better to have a static method and make the error message standardized. Not sure what you mean by getting the function name from the stack trace though. I was thinking to just pass the method name to the static helper method.
Do you want to do this cleanup in this JIRA, or address it separately in a follow-up?
There was a problem hiding this comment.
Let's do just the new two functions in this JIRA so we introduce the function.
Afterwards we can do a separate JIRA to replace all.
To get the method, one could do Thread.currentThread().getStackTrace()[index].getMethodName().
The index sometimes is tricky, but doable.
| /** Parameter name. */ | ||
| public static final String NAME = "namespacequota"; | ||
| /** Default parameter value ({@link Long#MAX_VALUE}). */ | ||
| // public static final String DEFAULT = String.valueOf(Long.MAX_VALUE); |
...p-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/resources/StorageSpaceQuotaParam.java
Show resolved
Hide resolved
| assertEquals(Long.valueOf(StorageSpaceQuotaParam.DEFAULT), | ||
| sp.getValue()); | ||
| sp = new StorageSpaceQuotaParam(100L); | ||
| assertEquals(new Long(100), sp.getValue()); |
| webHdfs.setQuota(path, -100, 100); | ||
| fail("Should have thrown exception"); | ||
| } catch (IllegalArgumentException e) { | ||
| assertTrue(e.getMessage().contains("Invalid values for quota")); |
There was a problem hiding this comment.
I would use LambdaTestUtils.intercept for all this.
There was a problem hiding this comment.
Thanks. I was searching for this but forgot what it is called. Will replace.
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
Hmm. Seems something wrong with Yetus? most of the tests failed with:
|
|
💔 -1 overall
This message was automatically generated. |
goiri
reviewed
Aug 9, 2019
| @@ -49,7 +51,7 @@ | |||
| @Test | |||
| public void testAccessTimeParam() { | |||
| final AccessTimeParam p = new AccessTimeParam(AccessTimeParam.DEFAULT); | |||
| Assert.assertEquals(-1L, p.getValue().longValue()); | |||
| assertEquals(-1L, p.getValue().longValue()); | |||
There was a problem hiding this comment.
To avoid so much churn, let's just do the assertEquals in the new methods and leave this alone for the rest for now.
we can do a followup JIRA for that if so.
There was a problem hiding this comment.
Sure. Will revert the unrelated changes in this class.
|
💔 -1 overall
This message was automatically generated. |
- Added a helper method for unsupported methods in FileSystem - Revert unrelated changes in TestParams
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.