Skip to content

[Feature Request] Implement a Config Loader #237

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

Closed
codevulture opened this issue May 29, 2017 · 9 comments
Closed

[Feature Request] Implement a Config Loader #237

codevulture opened this issue May 29, 2017 · 9 comments
Assignees
Labels
feature-request lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@codevulture
Copy link
Contributor

Current Implementation Of Client loads configuration from yaml File:

Following function loads yaml and do the processing.

https://github.com/kubernetes-client/python-base/blob/7c8e59b58d9a769f1ca3f818aa91c8a6538b51b1/config/kube_config.py#L309

_get_kube_config_loader_for_yaml_file

There are many cases and projects where Configurations are present in different options like ini file which needs to be loaded where creating a ConfigurationObject and assigning parameters is one way.It could save a lot of code if generic handler for loading could be implemented in Client-python as well.

If this looks feasible i can also volunteer to work on the same

@mbohlool
Copy link
Contributor

Sounds good to me. Can you provide more information about what is your plan to do? There is a Configuration class and current load_kube_config can be used to set. Maybe a sudo code structure of classes/functions that you plan to do helps me understand it. You can also just send the PR but was thinking we can discuss the structure first to have a faster code review.

@codevulture
Copy link
Contributor Author

Thanks, I will be posting the structure to discuss shortly.

@codevulture
Copy link
Contributor Author

Please find a draft version for implementation detail:

  • An Example is worth a thousand words:
    Sample file of a project using kubernetes having a section to declare parameters in it's local conf:
    Project.conf

.
.
[kubernetes]
api_root/base_url=127.0.0.1(url)
ssl_client_crt_file=file path
ssl_ca_crt_file=file path
ssl_verify_server_crt=file path
token_file=file_path

  • Assumption:
    Configuration File have to follow convention of parameters name in order to be loaded.

  • Proposal:
    So instead of Making a Configuration Object and assigning each parameter to configuration Object.
    We can have a INI type of file loader similar to implementation of yaml loader for projects:
    Like: Yaml Config Loader

@mbohlool
Copy link
Contributor

Thanks for providing the example. The part I am not clear on yet is if we save much code by generalizing config. Also the example is an ini file, what about the new code structure? What do you want to generalize?

@codevulture
Copy link
Contributor Author

@mbohlool :

The part I am not clear on yet is if we save much code by generalizing config.

  • If this was a case of single project there might not be a need to load params from ini, but taking the case of various projects doing the same thing might need some consideration. Sample

Also the example is an ini file, what about the new code structure? What do you want to generalize?

  • Code Structure would look something like:
def load_kube_config(config_file=None, context=None,
                     client_configuration=configuration):
    if config_file is None:
        config_file = os.path.expanduser(KUBE_CONFIG_DEFAULT_LOCATION)
    if config_file.endswith('.conf'): #Something like checking file, but this is just an assumption
        _get_kube_config_loader_for_ini_file(config_file,   active_context=context,client_configuration=client_configuration
        ).load_and_set()
    else:
        _get_kube_config_loader_for_yaml_file(config_file, active_context=context, client_configuration=client_configuration).load_and_set()

and similar to _get_kube_config_loader_for_yaml_file:

def _get_kube_config_loader_for_ini_file(filename, **kwargs):
    with open(filename) as f:
        return KubeConfigLoader(
            config_dict=load(f)  # find a way to load config,
            config_base_path=os.path.abspath(os.path.dirname(filename)),
**kwargs)

OR

maybe use a generic loader like:

def load_kube_config(config_file=None, context=None,
                     client_configuration=configuration):
    if config_file is None:
        config_file = os.path.expanduser(KUBE_CONFIG_DEFAULT_LOCATION)
    _get_kube_config_loader(config_file,   active_context=context,client_configuration=client_configuration
        ).load_and_set()

and implement loader based on file type (maybe ending with conf, or passing additional param to check whether file needs to be loaded in specific type.
and

def _get_kube_config_loader(filename, **kwargs):
    with open(filename) as f:
        return KubeConfigLoader(
            config_dict=load(f)  # find a way to load config as per type if passed as kwargs,
            config_base_path=os.path.abspath(os.path.dirname(filename)),
**kwargs)

I do not have an exact model at the moment but it would be good to hear from you about your thought on this. This may or may not be needed but it would be good to have.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 21, 2019
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 21, 2019
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

yliaog pushed a commit to yliaog/client-python that referenced this issue Jan 8, 2022
rename method _websocket_reqeust to _websocket_request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

4 participants