Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/526#issuecomment-93902909
I leaved some trivial comments. The patch looks good to me. It's a great
job.
Here is my additional comments. You try to change the signature
EvalNode::bind to take EvalContext instance in order to get an instance of a
launched script engine.
Even though this refactoring is already a breaking change. EvalNode still
need more information about task. In this chance, it would be great if we
refactor EvalNode to take more meta context including TajoConf, task attempt
information, and shared resources of workers. Then, we can probably remove
OverridableConf parameter from all constructors of EvalNode.
If you are concerned with a large patch, we can do this work in another
jira. But, it will cause the second breaking change. We can choose either two
breaking change or a large patch. It's up to you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
