[
https://issues.apache.org/jira/browse/YARN-8524?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16542731#comment-16542731
]
Szilard Nemeth edited comment on YARN-8524 at 7/13/18 8:59 AM:
---------------------------------------------------------------
Hi [~Zian Chen]!
1. Yes it's definitely better with a clarifying comment, added javadoc comment
to the constructor of ConfigurableResource.
For the rest of your comment: I think this patch was mainly about moving the
Resource/LightWeightResource code changes to Resources and give a meaningful
name to the new method, as per [~leftnoteasy] described in YARN-7556
I understand the API of ConfigurableResource could be even better but I think
what you described above can be achieved with the constructor that receives a
Resource object, so that users of this class have the freedom to initialize the
Resource as they want.
With that constructor, all the possible cases you mentioned are covered.
2. Added a new UT case
Please let me know if you still have concerns!
[~leftnoteasy]: I would like to have your opinion as well, do you think that
the patch is fine?
Thanks!
was (Author: snemeth):
Hi [~Zian Chen]!
1. Yes it's definitely better with a clarifying comment, added javadoc comment
to the constructor of ConfigurableResource.
For the rest of your comment: I think this patch was mainly about moving the
Resource/LightWeightResource code changes to Resources and give a meaningful
name to the new method, as per [~leftnoteasy] described in YARN-7556
I understand the API of ConfigurableResource could be even better but I think
what you described above can be achieved with the constructor that receives a
Resource object, so that users of this class have the freedom to initialize the
Resource as they want.
With thay constructor, all the possible cases you mentioned are covered.
2. Added a new UT case
Please let me know if you still have concerns!
[~leftnoteasy]: I would like to have your opinion as well, do you think that
the patch is fine?
Thanks!
> Single parameter Resource / LightWeightResource constructor looks confusing
> ---------------------------------------------------------------------------
>
> Key: YARN-8524
> URL: https://issues.apache.org/jira/browse/YARN-8524
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: api
> Reporter: Szilard Nemeth
> Assignee: Szilard Nemeth
> Priority: Major
> Attachments: YARN-8524.001.patch, YARN-8524.002.patch
>
>
> The single parameter (long) constructor in Resource / LightWeightResource
> sets all resource components to the same value.
> Since there are other constructors in these classes with (long, int)
> parameters where the semantics are different, it could be confusing for the
> users.
> The perfect place to create such a resource would be in the Resources class,
> with a method named like "createResourceWithSameValue".
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
