Refactoring of SPI #1051
Closed
Refactoring of SPI #1051
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
| @@ -44,12 +48,15 @@ | |||
|
|
|||
| public class ConnectorManager | |||
There was a problem hiding this comment.
In general, if any of the *Manager.add*Manager() methods fail, you need to remove the mapping from the managers already modified. Otherwise, you can leave the managers in a corrupt state. I suggest the methods be of the shape *Manager.remove*Manager(String connectorId, ConnectoManager expectedManager)
There was a problem hiding this comment.
That is the case with the current code, too. I agree, it should be fixed, but I'd rather do it as separate change. This one already has too much stuff in it.
|
This is a mostly mind numbing mechanical refactoring, so if there are specific sections you want reviewed in more detail, just ask. Otherwise, looks good |
This is a precursor to splitting the *Handle hierarchy so that connectors don't need to be aware of connector ids
Some tests were incorrectly using MetadataManager.INTERNAL_CONNECTOR_ID
There are now two parallel Handle hierarchies: global vs connector-specific. SPI operates in terms of the connector-specific hierarchy. The engine (analyzer, planner), operates in terms of the global hierarchy. Metadata/SplitManager/etc take care of wrapping and unwrapping calls and routing to the appropriate connector based on the connectorId embedded in the global handles.
…ectors They are all now treated as internal connectors. ConnectorManager is now responsible for installing the information schema metadata when appropriate
LocalQueryRunner was installing the metadata, split manager and data stream provider manually instead of relying on the existing SystemConnector and DualConnectors
|
merged |
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.
This is a preliminary pull request to simplify the SPI. The goal is to no longer require connectors to have to implement canHandle(...) and test for connectorId, which is repetitive and error prone.
I haven't yet removed the HandleResolvers due to a couple of bugs in Jackson:
FasterXML/jackson-databind#406
FasterXML/jackson-databind#408