HubAppDeployer no longer loses functionality in SimpleAppDeployer #1700
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HubAppDeployer now extends SimpleAppDeployer in a way that it doesn't lose any functionality.
rjrudin
requested review from
aebadirad,
srinathgit,
akshaysonvane and
bsrikan
|
The PR looks good. Should DataHubImpl.updateIndexes() be changed to use HubAppDeployer as part of this PR ? |
|
It'd be good to do that, yes. |
|
AFAICT, there's no reason to use HubAppDeployer for updating indexes - unless the "telemetry" call in HubAppDeployer matters for this operation. But my hunch is that it doesn't, because that telemetry stuff seems specific to installs, not partial operations like updating indexes. Which makes me think it's better that SimpleAppDeployer is used instead of HubAppDeployer. |
srinathgit
approved these changes
Dec 19, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
HubAppDeployer now extends SimpleAppDeployer in a way that it doesn't lose any functionality.
This is not intended for the 4.1.0 release - hoping it can go into 4.1.1 or 4.2.x.