-
Notifications
You must be signed in to change notification settings - Fork 6
Client Config Builder Module (Refactored) #54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ng structure of CCB package
…esolvers, wrappers )
packages/polywrap-client-config-builder/polywrap_client_config_builder/client_config_builder.py
Outdated
Show resolved
Hide resolved
Niraj-Kamdar
left a comment
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.
The implementation isn't according to the latest js changes. Please update it to be in parity with js client
packages/polywrap-client-config-builder/polywrap_client_config_builder/client_config_builder.py
Outdated
Show resolved
Hide resolved
packages/polywrap-client-config-builder/polywrap_client_config_builder/client_config_builder.py
Outdated
Show resolved
Hide resolved
packages/polywrap-client-config-builder/polywrap_client_config_builder/client_config_builder.py
Outdated
Show resolved
Hide resolved
packages/polywrap-client-config-builder/polywrap_client_config_builder/client_config_builder.py
Outdated
Show resolved
Hide resolved
packages/polywrap-client-config-builder/polywrap_client_config_builder/client_config_builder.py
Outdated
Show resolved
Hide resolved
packages/polywrap-client-config-builder/polywrap_client_config_builder/client_config_builder.py
Outdated
Show resolved
Hide resolved
packages/polywrap-client-config-builder/polywrap_client_config_builder/client_config_builder.py
Outdated
Show resolved
Hide resolved
packages/polywrap-client-config-builder/polywrap_client_config_builder/client_config_builder.py
Outdated
Show resolved
Hide resolved
packages/polywrap-client-config-builder/polywrap_client_config_builder/client_config_builder.py
Outdated
Show resolved
Hide resolved
|
I've pushed progress again, adding more through testing, cleanups, fixtures, removing abstractclass decorators from functions with an implementation, refactored the client_config.packages property, added new tests for it, and also a first e2e test of the generic ccb.add(partial_client_config) |
, test:e2e test of generic add configurations to CCB , remove unnecessary functions from ClientConfigBuilder class
|
Applied lint changes and new requests from Cesar, including the redirects and its functions |
Niraj-Kamdar
left a comment
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.
Checkout https://github.com/polywrap/toolchain/tree/origin-dev/packages/js/client-config-builder since it's been changed a lot after this PR.
Some notable changes:
- BuilderConfig
- Only CoreClientConfig remained
- build signature changed
Niraj-Kamdar
left a comment
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.
Thanks @rihp for the contributions! I have disabled the test for now but will be re-enabling when I tackle exhaustive test tasks since it requires changes due to refactors!
Continued from #53, enables the Client Config Builder Module following the requests from @cbrzn
The new implementation uses the
ClientConfigas a dataclass to later create an Abstract Base Class of aBaseClientConfigBuilderwhich defines more clearly the functions of the module, like add_envs, set_resolvers, remove_wrappers, and so on.This
BaseClientConfigBuilderis later used in the classClientConfigBuilderwhich only implements the build method, for now, and inherits all the abstract methods of theBaseClientConfigBuilder.Features