[
https://issues.apache.org/jira/browse/YARN-5184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15625867#comment-15625867
]
Sangjin Lee commented on YARN-5184:
-----------------------------------
{quote}
What if the methods in question are used by rest of YARN code? If someone
subclasses this to provide an alternate implementation, calling these methods
would lead to RTE.
{quote}
Yes, I agree that it is a very valid concern, as a runtime exception is a poor
substitute for a compile-time error. I think a mitigating circumstance here is
that these classes are normally extended only by the *canonical* implementation
(protobuf impl): {{ContainerStatusPBImpl}} for {{ContainerStatus}}, and
{{NodeReportPBImpl}} for {{NodeReport}}. Thus, the real likelihood that these
classes would be extended by other classes would be pretty small, but it cannot
be ruled out. In practice, the only times these are extended might be test
classes (mock classes).
In that sense, actually it could be an argument *for* not moving this change to
the trunk. By doing so, we would be extending this window of potential trouble
into 3.0. Since 2.8 has not been released, we could consider this as adding
compatible default implementation methods which will then be made abstract in
3.0. Thoughts?
{quote}
The reason I am raising this concern is to see if there is a need for one more
annotation that clarifies our intent about users extending classes to be
plugged in.
{quote}
I do agree that we probably need another audience annotation for whether users
may extend/implement classes (which I also proposed in the original email
thread):
{quote}
It could be feasible to have an interface as Public/Stable for users (anyone
can use the API in a stable manner) but Private for implementers. The idea is
that it is still a public interface but no third-party code should not subclass
or implement it. I suspect a fair amount of hadoop's public interface might
fall into that category. That itself is probably an incompatible change, so we
might have to wait until after 3.0, however.
{quote}
One practical difficulty I see is for test classes. One key need to
extend/implement the interface is because users need to provide a mocked test
class. If we declare these interfaces are off limits to implement/extend, then
they would need to stick with pure mocking via mocking libraries. I'm not sure
how onerous that requirement would be.
cc [[email protected]] for his thoughts.
> Fix up incompatible changes introduced on ContainerStatus and NodeReport
> ------------------------------------------------------------------------
>
> Key: YARN-5184
> URL: https://issues.apache.org/jira/browse/YARN-5184
> Project: Hadoop YARN
> Issue Type: Bug
> Components: api
> Affects Versions: 2.9.0
> Reporter: Karthik Kambatla
> Assignee: Sangjin Lee
> Priority: Blocker
> Attachments: YARN-5184-branch-2.8.poc.patch,
> YARN-5184-branch-2.poc.patch
>
>
> YARN-2882 and YARN-5430 broke compatibility by adding abstract methods to
> ContainerStatus. Since ContainerStatus is a Public-Stable class, adding
> abstract methods to this class breaks any extensions.
> To fix this, we should add default implementations to these new methods and
> not leave them as abstract.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
