Refactor search to get rid of dependency on static api_client instance
#468
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
jacksonj04
force-pushed
the
chore/refactor-apiclient-out-of-search
branch
from
18b4edc to
d6456f9
Compare
Previously, `SearchResultMetadata` had a static `create_from_uri` method which was being called by `SearchResult`, which then ultimately initialised an instance of `SearchResultMetadata`. However, this is the _only_ place where this method was being called from. This means we can push this method upwards into `SearchResult` and avoid the need for a class acting as its own factory.
This method was making `SearchResponse` its own factory, but was only ever being used to perform XML parsing. We can move this back up the stack to clarify what is going on here.
jacksonj04
force-pushed
the
chore/refactor-apiclient-out-of-search
branch
from
93997f7 to
b9a08ee
Compare
Search, specifically metadata handling, was relying on the static instance of api_client which was initialised as part of loading this API library. This instance is due to be deprecated, which means that instead the dynamic instance which is initialised by the calling code must be passed back down the stack.
jacksonj04
changed the title
api_client instanceapi_client instance, and then remove it
jacksonj04
force-pushed
the
chore/refactor-apiclient-out-of-search
branch
from
00846de to
077b646
Compare
jacksonj04
changed the title
api_client instance, and then remove itapi_client instance
timcowlishaw
approved these changes
Nov 29, 2023
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.
Searching currently relies on the static instance of
api_clientwhich is initialised atClientload-time to interface with the database and retrieve metadata.This instance is deprecated and due for removal, so we need to fix this behaviour. Search should only ever use the
Clientinstance it was called with, so we need to either inject the dependency down or refactor the behaviour so it can directly access theClient.As part of this, we move factory methods out of
SearchResultMetadataandSearchResult, which provides a cleaner separation of concerns and reduces the depth to which we need to make changes to account for removingapi_client.There should be no functional changes as a result of this PR, and it should be non-breaking for EUI and PUI.