[GitHub] [incubator-gobblin] jhsenjaliya commented on issue #2691: [GOBBLIN-830] launcher.type to job.launcher.type
jhsenjaliya commented on issue #2691: [GOBBLIN-830] launcher.type to job.launcher.type URL: https://github.com/apache/incubator-gobblin/pull/2691#issuecomment-596905586 @arjun4084346, @sv2000 can u pls take a look? Thanks This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on issue #2691: [GOBBLIN-830] launcher.type to job.launcher.type
jhsenjaliya commented on issue #2691: [GOBBLIN-830] launcher.type to job.launcher.type URL: https://github.com/apache/incubator-gobblin/pull/2691#issuecomment-528501611 @arjun4084346 , i made whatever changes i had to make to facilitate the backward compatibility, please take a look. Thanks This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on issue #2691: [GOBBLIN-830] launcher.type to job.launcher.type
jhsenjaliya commented on issue #2691: [GOBBLIN-830] launcher.type to
job.launcher.type
URL:
https://github.com/apache/incubator-gobblin/pull/2691#issuecomment-527246779
@arjun4084346 , tried to handle both properties but its getting complex
- {JobLauncherType} is nested, need to taken out, since most of the
jobLauncher is defined in gobblin-runtime, it needs to be placed under that
module.
- instead of handling if/else at multiple place, I should add
{JobLauncherUtils.getJobLauncherType} method, but that would create circular
dependency between gobblin-utility and gobblin-runtime.
This is minor change compare to amount of refactoring that is required to
support both properties which is basically touching the complexity of the
project that requires proper refactoring (
[GIP2](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=119547565)
)
Any thoughts/comments ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on issue #2691: [GOBBLIN-830] launcher.type to job.launcher.type
jhsenjaliya commented on issue #2691: [GOBBLIN-830] launcher.type to job.launcher.type URL: https://github.com/apache/incubator-gobblin/pull/2691#issuecomment-515707216 the reason for this change is standardization or making it clear, when i came across this property, I had to look up the code to make sure whether it really means job launcher of something else. there are some configuration keys that are not standardized or having misleading names, that would need to be updated eventually. thought this would be an easy change while upgrading, may be i can just put both and mark this for deprecation, would that work for you guys? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
