FALCON-2200 Update API support for extension job (user extension) #331
Conversation
…ters being added into entity definition
if (StringUtils.isNotBlank(extensionName)) { | ||
extensionDetailJson = getExtensionDetailJson(extensionName); | ||
} else { | ||
extensionDetailJson = getExtensionJobDetailJson(jobName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would prefer to keep this cleaner by making a call to getExtensionJobDetails and then another to getExtensionDetail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will make the change to make 2 api calls.
@@ -121,6 +122,8 @@ private JSONObject buildExtensionJobDetailResult(final String jobName) throws Fa | |||
detailsObject.put(CONFIG, jobsBean.getConfig()); | |||
detailsObject.put(CREATION_TIME, jobsBean.getCreationTime()); | |||
detailsObject.put(LAST_UPDATE_TIME, jobsBean.getLastUpdatedTime()); | |||
detailsObject.put(EXTENSION_LOCATION, extensionBean.getLocation()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not part of the details of the Job. Lets not add this here. Instead lets have update make 2 calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will make the change to make 2 api calls.
|
||
for (Map.Entry<EntityType, List<Entity>> entry : entityMap.entrySet()) { | ||
for (final Entity entity : entry.getValue()) { | ||
final String entityType = entity.getEntityType().toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This repeats code from SchedulableEntityManagerProxy. Any changes/fixes will need to be made to 2 places. Can you consider introducing a Util class and using it in both SchedulableEntityManagerProxy and ExtensionManagerProxy? The problem existed when submit and submitAndSchedule were introduced. As we introduce more APIs, have repetition will get worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made the chcanges accordingly
…ies to proxyutil and making 2 api calls to get location in case of update extension
|
||
for (Map.Entry<EntityType, List<Entity>> entry : entityMap.entrySet()) { | ||
for (final Entity entity : entry.getValue()) { | ||
final String entityType = entity.getEntityType().toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I will move the submit and update Apis to a new util.
if (StringUtils.isNotBlank(extensionName)) { | ||
extensionDetailJson = getExtensionDetailJson(extensionName); | ||
} else { | ||
extensionDetailJson = getExtensionJobDetailJson(jobName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will make the change to make 2 api calls.
@@ -121,6 +122,8 @@ private JSONObject buildExtensionJobDetailResult(final String jobName) throws Fa | |||
detailsObject.put(CONFIG, jobsBean.getConfig()); | |||
detailsObject.put(CREATION_TIME, jobsBean.getCreationTime()); | |||
detailsObject.put(LAST_UPDATE_TIME, jobsBean.getLastUpdatedTime()); | |||
detailsObject.put(EXTENSION_LOCATION, extensionBean.getLocation()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will make the change to make 2 api calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have these changes been dev-tested in both embedded and distributed mode?
@@ -96,8 +96,9 @@ | |||
|
|||
private boolean embeddedMode = DeploymentUtil.isEmbeddedMode(); | |||
private String currentColo = DeploymentUtil.getCurrentColo(); | |||
private final Map<String, Channel> configSyncChannels = new HashMap<String, Channel>(); | |||
private final Map<String, Channel> entityManagerChannels = new HashMap<String, Channel>(); | |||
private final Map<String, Channel> configSyncChannels = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we still need these 2 variables if we are using the the EntityProxyUtil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are not required. Missed to removed them.
I have tested them in distributed mode. For embedded mode the unit tests run from FalconCLIIT so that should be sufficient |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
…l from ExtensionManager Proxy as they are now used from proxyUtil
This pull request is dependent on #331 Author: sandeep <sandysmdl@gmail.com> Reviewers: @pallavi-rao Closes #333 from sandeepSamudrala/FALCON-2199
Author: sandeep <sandysmdl@gmail.com> Reviewers: @pallavi-rao Closes apache#331 from sandeepSamudrala/FALCON-2200 and squashes the following commits: 737fad3 [sandeep] FALCON-2200 fixed checkstyle issues. removed unused imports 1780416 [sandeep] Incorporated review comments. Removed entitychannel and config channel from ExtensionManager Proxy as they are now used from proxyUtil 8a4d035 [sandeep] FALCON-2200 Incorporated review comments. Moved common code from proxies to proxyutil and making 2 api calls to get location in case of update extension c8d0ab7 [sandeep] FALCON-2200 Adding changes related to clusters being removed and clusters being added into entity definition cc7c9e9 [sandeep] FALCON-2200 Update API support for extension job (user extension) 456d4ee [sandeep] Merge branch 'master' of https://github.com/apache/falcon 0cf9af6 [sandeep] Merge branch 'master' of https://github.com/apache/falcon 4a2e23e [sandeep] Merge branch 'master' of https://github.com/apache/falcon b1546ed [sandeep] Merge branch 'master' of https://github.com/apache/falcon 0a433fb [sandeep] Merge branch 'master' of https://github.com/apache/falcon 194f36a [sandeep] Merge branch 'master' of https://github.com/apache/falcon e0ad358 [sandeep] Merge branch 'master' of https://github.com/apache/falcon f96a084 [sandeep] Merge branch 'master' of https://github.com/apache/falcon 9cf36e9 [sandeep] Merge branch 'master' of https://github.com/apache/falcon bbca081 [sandeep] Merge branch 'master' of https://github.com/apache/falcon 48f6afa [sandeep] Merge branch 'master' of https://github.com/apache/falcon 250cc46 [sandeep] Merge branch 'master' of https://github.com/apache/falcon d0393e9 [sandeep] Merge branch 'master' of https://github.com/apache/falcon a178805 [sandeep] Merge branch 'master' of https://github.com/apache/falcon d6dc8bf [sandeep] Merge branch 'master' of https://github.com/apache/falcon 1bb8d3c [sandeep] Merge branch 'master' of https://github.com/apache/falcon c065566 [sandeep] reverting last line changes made 1a4dcd2 [sandeep] rebased and resolved the conflicts from master 271318b [sandeep] FALCON-2097. Adding UT to the new method for getting next instance time with Delay. a94d4fe [sandeep] rebasing from master 9e68a57 [sandeep] FALCON-298. Feed update with replication delay creates holes
This pull request is dependent on apache#331 Author: sandeep <sandysmdl@gmail.com> Reviewers: @pallavi-rao Closes apache#333 from sandeepSamudrala/FALCON-2199
No description provided.