Major refactor of Shillelagh adapter implementation #43
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
the cardholders endpoints were defined in cc.cardholders.cardholders, while i have been implementing the alarm endpoints i have kept them in the top level package and the others in subpackages, this structure is mostly to corelate to how the documentaiton is presented by gallagher
the helper needs to be initialised after the api key has been set hence moving this to the adapter class and not the cc helper, the logic is still maintained in the cc helper vtable package but executed in the adapter class at the moment various things are being initialised on the package level this should be reviwed before we merge things into master
an extremely rough implementation of auto discovering column names from the matching DTO class, this introduces a sql_model attribute to the DTO configuration for getting things going. TODO: this has to be cleaned up and should not ship this way things to check: - read the DTO form the summary results - cache properties where possible for performance - revise how field type are instantiated when mapping occurs
since the original incarnation i have introduced responses and payloads this is due to the project evolving and these should be documented in design.md
while browsing some docs i found the use of highlights https://github.com/orgs/community/discussions/16925 which i've migrated to using
the vtables package contains code that would sit directly in the adapter, so i am moving things directly into the adapter, moreover i thik that the core package sould be free of anything to do with shillelagh and hence moving as much shillelagh related things out of there as possible
initially i was implementing a sql interface within the dto objects, as the solution grew it made more sense to be able to consolidate this into the shillelagh extension, ensuring that if the sql interface isn't in use the normal dto isn't burdened by imports i've kept some helper methods to concat dictionaries of annotations, this should not affect the workings of the solution refs #31
result_set wraps the property from the class that should be returned this is because each summary or response has a different collection variable e.g cardholder has results where as alarms has alarms the wrapper is called result_set to signify how a sql orm or wrapper would refer to the results
using the detail dto_retrieve model to parse the sql responses returned to the shillelagh, things to check post this commit - can we get the detail fields when the user requests a model via where clause - is this the right thing to do (moving away from using the sql_model configuration) removes various TODO markers as appropriate where the issues have been resolved refs #31
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.
The branch has a major refactor of the way Shillelagh adapters are implemented in the project, this primarily covers:
2.10) #20 of responses)