You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kibble.apache.org by GitBox <gi...@apache.org> on 2020/10/11 11:20:47 UTC
[GitHub] [kibble] turbaszek opened a new issue #52: Refactor the configuration methods of Apache Kibble
turbaszek opened a new issue #52:
URL: https://github.com/apache/kibble/issues/52
Hello,
Currently, Apache Kibble is configured via `kibble.yaml` which is generated by running the `setup.py` script. This file is referenced in multiple places in the codebase which is not necessary in my opinion.
I would like to propose to use for the standard Python library `configparser ` instead of yaml.
https://docs.python.org/3.7/library/configparser.html
So instead of:
```yaml
accounts:
allowSignup: true
verify: true
api:
database: 2
version: 0.1.0
elasticsearch:
dbname: kibble
host: elasticsearch
port: 9200
ssl: false
mail:
mailhost: localhost
mailport: 25
sender: Kibble <no...@kibble.kibble>
```
we would have:
```ini
[accounts]
allowSignup = True
verify = True
[api]
database = 2
version = 0.1.0
[elasticsearch]
dbname = kibble
host = elasticsearch
port = 9200
ss = false
[mail]
mailhost = localhost
mailport = 25
sender = Kibble <no...@kibble.kibble>
```
The main advantage of using configparser is that we will be able to parse the config file once and then access any value doing something like `config.get("section", "key")`.
Additionally, we may take some inspiration from Apache Airflow project and:
- introduce "default config" that will be used for fallback values that were not defined by user
- use env variables to override the config (dead useful for configuring environments) for example `export KIBBLE__MAIL__MAILPORT=34` would override the config value of mail.mailport
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kibble] skekre98 commented on issue #52: Refactor the configuration methods of Apache Kibble
Posted by GitBox <gi...@apache.org>.
skekre98 commented on issue #52:
URL: https://github.com/apache/kibble/issues/52#issuecomment-715456600
Cool, I can start building up the class. Based on my current understanding we could initialize the parser with _kibble.yaml_ in `__init__()` followed by another getter function:
```python
class KibbleConfigParser(...):
def __init__():
# initialize with kibble.yaml
def get(section, key):
# export variable
```
Is my understanding along the lines of what you are looking for?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kibble] turbaszek commented on issue #52: Refactor the configuration methods of Apache Kibble
Posted by GitBox <gi...@apache.org>.
turbaszek commented on issue #52:
URL: https://github.com/apache/kibble/issues/52#issuecomment-715078800
@skekre98 thanks for help!
> Do you want refactor `../setup/setup.py` to use `configparser` instead or `yaml`?
Yes, I think this would be more python-native solution. However, I would suggest to add this `setup/configuration.py` file where we do:
```
class KibbleConfigParser(...):
...
conf = KibbleConfigParser()
```
Then we can import the `conf` anywhere and access the configuration information. But I'm open to any suggestion.
One more thing: I think it would be simpler to merge dbname, host and mort of elastiscearch into single connection uri field. WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kibble] Humbedooh commented on issue #52: Refactor the configuration methods of Apache Kibble
Posted by GitBox <gi...@apache.org>.
Humbedooh commented on issue #52:
URL: https://github.com/apache/kibble/issues/52#issuecomment-715085796
> @skekre98 thanks for help!
>
> > Do you want refactor `../setup/setup.py` to use `configparser` instead or `yaml`?
>
> Yes, I think this would be more python-native solution. However, I would suggest to add this `setup/configuration.py` file where we do:
>
> ```
> class KibbleConfigParser(...):
> ...
>
> conf = KibbleConfigParser()
> ```
>
> Then we can import the `conf` anywhere and access the configuration information. But I'm open to any suggestion.
>
> One more thing: I think it would be simpler to merge dbname, host and mort of elastiscearch into single connection uri field. WDYT?
Definitely +1 to merging the entire URL (I think I said the same thing earlier somewhere else), that will also make it easier to auto-configure via the command line environment, as you'll just be exporting one variable :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kibble] turbaszek closed issue #52: Refactor the configuration methods of Apache Kibble
Posted by GitBox <gi...@apache.org>.
turbaszek closed issue #52:
URL: https://github.com/apache/kibble/issues/52
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kibble] skekre98 commented on issue #52: Refactor the configuration methods of Apache Kibble
Posted by GitBox <gi...@apache.org>.
skekre98 commented on issue #52:
URL: https://github.com/apache/kibble/issues/52#issuecomment-714831722
@turbaszek I am interested in taking this up. As clarification, how would you like this to be incorporated? Do you want refactor `../setup/setup.py` to use `configparser` instead or `yaml`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kibble] turbaszek commented on issue #52: Refactor the configuration methods of Apache Kibble
Posted by GitBox <gi...@apache.org>.
turbaszek commented on issue #52:
URL: https://github.com/apache/kibble/issues/52#issuecomment-715889419
@skekre98 I think that python native ConfigParser does not support yamls out of the box. But it may be possible to construct ConfigParser from dictionary - however, I'm not sure if this is worth having additional custom logic.
I think keeping it simple as possible would be the best option. So:
1. Use `ini` type of file instead of yaml: https://docs.python.org/3.7/library/configparser.html#supported-ini-file-structure
2. And then:
```
config = configparser.ConfigParser()
config.read("kibble.ini")
```
However, I'm open to any other suggestion
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kibble] skekre98 commented on issue #52: Refactor the configuration methods of Apache Kibble
Posted by GitBox <gi...@apache.org>.
skekre98 commented on issue #52:
URL: https://github.com/apache/kibble/issues/52#issuecomment-716031341
@turbaszek yes, I definitely think keeping in `ini` format will work better. Misunderstanding on my part 😅
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org