Use YAML format for tanner configuration #371

mzfr · 2020-03-09T14:36:01Z

Fixes #366

Few things might fail but I am working on this. Made the PR so you(@afeena) can take a look and give your input about the way it's done.

Since the old config was using staticmethod decorator I decided to use those

@staticmethod

This has to be done so TravisCI can include a file ## To make a commit, type your commit message and press CTRL-ENTER. ## To cancel the commit, close the window. To sign off on the commit, ## press CTRL-S. ## ## You may also reference or close a GitHub issue with this commit. ## To do so, type `#` followed by the `tab` key. You will be shown a ## list of issues related to the current repo. You may also type ## `owner/repo#` plus the `tab` key to reference an issue in a ## different GitHub repo. setup.py | 3 ++- tanner/config.py | 4 ++-- tanner/{ => data}/config.yaml | 0 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/setup.py b/setup.py index edcecad..01aa755 100644 --- a/setup.py +++ b/setup.py @@ -12,5 +12,6 @@ setup(name='Tanner', scripts=['bin/tanner', 'bin/tannerweb', 'bin/tannerapi'], data_files=[('/opt/tanner/db/', ['tanner/data/db_config.json', 'tanner/data/GeoLite2-City.mmdb']), ('/opt/tanner/data/', ['tanner/data/dorks.pickle', 'tanner/data/crawler_user_agents.txt', - 'tanner/files/engines/mako.py', 'tanner/files/engines/tornado.py'])] + 'tanner/files/engines/mako.py', 'tanner/files/engines/tornado.py', + 'tanner/data/config.yamls'])] ) diff --git a/tanner/config.py b/tanner/config.py index c5aa423..788a56e 100644 --- a/tanner/config.py +++ b/tanner/config.py @@ -18,7 +18,7 @@ class Meta(type): def parse_default_configs(path): return read_config(path) - default_config = parse_default_configs("/opt/tanner/config.yaml") + default_config = parse_default_configs("/opt/tanner/data/config.yaml") attribs.update({ 'default_config': default_config, 'parse_default_configs': parse_default_configs @@ -32,7 +32,7 @@ class TannerConfig(metaclass=Meta): @staticmethod def set_default_config(default_config_path): - TannerConfig.default_config = read_config("/opt/tanner/config.yaml") + TannerConfig.default_config = read_config(default_config_path) @staticmethod def set_config(config_path): diff --git a/tanner/config.yaml b/tanner/data/config.yaml similarity index 100% rename from tanner/config.yaml rename to tanner/data/config.yaml

coveralls · 2020-03-09T18:34:50Z

Pull Request Test Coverage Report for Build 1233

15 of 19 (78.95%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.7%) to 76.847%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
tanner/config.py	14	18	77.78%

Totals
Change from base Build 1231:	0.7%
Covered Lines:	1404
Relevant Lines:	1827

💛 - Coveralls

mzfr · 2020-03-09T18:37:40Z

@afeena Finally all the tests are passing so please review it.

afeena · 2020-03-09T21:07:08Z

config.ini

@@ -0,0 +1,2 @@
+[API]


why you need this file? Don't we want to move to yml?

Sorry, this was by mistake. Since we are going to use YAML config file I've removed this from the source.

afeena · 2020-03-09T22:00:09Z

tanner/config.py

+    return config_values
+
+
+class Meta(type):


why you need meta class? which problem does it solve?

I was using Meta class to set the default value of the config within the class.
But now I realized it would be just fine if we can stick to the old model of config i.e using a GLOBAL variable to hold the default values.

afeena · 2020-03-10T21:57:50Z

tanner/config.py

-                   'SESSIONS': {"delete_timeout": 300}
-                   }
+
+def read_config(path):


you can make this function also a class function

afeena · 2020-03-10T21:58:19Z

tanner/config.py

+    return config_values
+
+
+DEFAULT_CONFIG = read_config("/opt/tanner/data/config.yaml")


why do not read default while setting config?

afeena · 2020-03-10T22:02:28Z

tanner/config.py

-                LOGGER.warning("Error in config, default value will be used. Section: %s Value: %s", section, value)
-                res = config_template[section][value]
-
+                res = TannerConfig.config[section][value]


instead of using if/try you can use noly try and catch also TypeError (case TannerConfig.config is None)

or

you can merge together default config and user config

afeena · 2020-03-10T22:03:47Z

Looks good :) I tested it a little bit and it works well, just some minor comments :)

mzfr · 2020-03-11T18:48:47Z

@afeena The suggested changes have been made.

afeena · 2020-03-11T19:39:48Z

If I try to set only one particular value through config, I get an error:

Traceback (most recent call last):
  File "/usr/local/bin/tanner", line 4, in <module>
    __import__('pkg_resources').run_script('Tanner==0.6.0', 'tanner')
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 661, in run_script
    self.require(requires)[0].run_script(script_name, ns)
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 1448, in run_script
    exec(script_code, namespace, namespace)
  File "/usr/local/lib/python3.7/site-packages/Tanner-0.6.0-py3.7.egg/EGG-INFO/scripts/tanner", line 35, in <module>
  File "/usr/local/lib/python3.7/site-packages/Tanner-0.6.0-py3.7.egg/EGG-INFO/scripts/tanner", line 30, in main
  File "/usr/local/lib/python3.7/site-packages/Tanner-0.6.0-py3.7.egg/tanner/server.py", line 31, in __init__
  File "/usr/local/lib/python3.7/site-packages/Tanner-0.6.0-py3.7.egg/tanner/emulators/base.py", line 19, in __init__
KeyError: 'rfi'

my config file:

EMULATOR_ENABLED: 
  lfi: False

afeena · 2020-03-11T19:41:06Z

tanner/config.py

-                   'SESSIONS': {"delete_timeout": 300}
-                   }
+
+class ReadConfig():


maybe it's better to move read_config in tanner config class? Have a separate class for only one function which is logically connected with other general class seems redundant for me

Parth1811 · 2020-03-11T20:50:29Z

Hey, won't this make the old config files obsolete?

mzfr · 2020-03-12T02:27:01Z

@afeena Yes, that is because in base.py we are using TannerConfig.get_section to grab EMULATOR_ENABLED section, which in our case is present but doesn't have all the values.

One solution is that we have a kind of check function, like if the section doesn't have all the values then the missing values will be added from the default config.

@Parth1811 Yes, I think that was the idea.

afeena · 2020-03-12T22:00:25Z

@mzfr my point is, that it would be nice to have this property, just e.g. I want to switch off one particular emulator, so I don't want to define al others as true. I suggest you merge two dict (might be a problem for nested structure), in this case, you can keep only one copy of config and don't do additional checks. Or maybe you have another good solution :)

And as @Parth1811 mention, the old config will be not supported anymore, so it would be nice to catch the exception while trying to load yml config (in case someone wants to load ini file)

Also added a check for old config format

mzfr · 2020-03-13T17:39:04Z

@afeena I've made the changes that we discussed. Also, I have updated the config docs.

Let me know if something is still missing out.

afeena · 2020-03-16T16:01:39Z

tanner/config.py

+    def read_config(path):
+        config_values = {}
+        with open(path, 'r') as f:
+            config_values = yaml.load(f, Loader=yaml.FullLoader)


I think it would be nice to catch the exception instead of checking the ending of filename here https://github.com/mushorg/tanner/pull/371/files#diff-f428ee775d6702ef490b2120c5ef2ae5R23, because e.g. I've never used .ini for configs, I used .config :)

afeena · 2020-03-16T16:03:49Z

docs/source/config.rst

-     'LOCALLOG': {'enabled': 'False', 'PATH': '/tmp/tanner_report.json'},
-     'CLEANLOG': {'enabled': 'False'}
-     }
+  DATA:


You can use comments to describe a little bit sections :)
I find this example quite good :)
https://opennmt.net/OpenNMT-tf/configuration.html

Also added some comment for config docs. And removed a comma from a config

afeena · 2020-03-17T16:57:50Z

@mzfr Awesome! It's ready to merge, since it contains breaking changes, what do you think if we merge it into develop first together with upcoming docker and dir changes and then prepare new release?

mzfr · 2020-03-17T17:15:00Z

@afeena yeah that would be a good thing to do.

afeena · 2020-03-18T15:02:42Z

@mzfr could you please then change destination to develop branch?

mzfr · 2020-03-18T15:38:07Z

@afeena Done. And luckily I don't have to rebase :)

afeena · 2020-03-18T15:39:27Z

@mzfr great, thanks!

@staticmethod

* Use YAML format for tanner configuration * Add pyyaml to requirements.txt * remove usage of self in TannerConfig class Since the old config was using staticmethod decorator I decided to use those * Fix the format of YAML config * Use metaclass to setup default values * Update config test * place default config file under tanner/data This has to be done so TravisCI can include a file ## To make a commit, type your commit message and press CTRL-ENTER. ## To cancel the commit, close the window. To sign off on the commit, ## press CTRL-S. ## ## You may also reference or close a GitHub issue with this commit. ## To do so, type `#` followed by the `tab` key. You will be shown a ## list of issues related to the current repo. You may also type ## `owner/repo#` plus the `tab` key to reference an issue in a ## different GitHub repo. setup.py | 3 ++- tanner/config.py | 4 ++-- tanner/{ => data}/config.yaml | 0 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/setup.py b/setup.py index edcecad..01aa755 100644 --- a/setup.py +++ b/setup.py @@ -12,5 +12,6 @@ setup(name='Tanner', scripts=['bin/tanner', 'bin/tannerweb', 'bin/tannerapi'], data_files=[('/opt/tanner/db/', ['tanner/data/db_config.json', 'tanner/data/GeoLite2-City.mmdb']), ('/opt/tanner/data/', ['tanner/data/dorks.pickle', 'tanner/data/crawler_user_agents.txt', - 'tanner/files/engines/mako.py', 'tanner/files/engines/tornado.py'])] + 'tanner/files/engines/mako.py', 'tanner/files/engines/tornado.py', + 'tanner/data/config.yamls'])] ) diff --git a/tanner/config.py b/tanner/config.py index c5aa423..788a56e 100644 --- a/tanner/config.py +++ b/tanner/config.py @@ -18,7 +18,7 @@ class Meta(type): def parse_default_configs(path): return read_config(path) - default_config = parse_default_configs("/opt/tanner/config.yaml") + default_config = parse_default_configs("/opt/tanner/data/config.yaml") attribs.update({ 'default_config': default_config, 'parse_default_configs': parse_default_configs @@ -32,7 +32,7 @@ class TannerConfig(metaclass=Meta): @staticmethod def set_default_config(default_config_path): - TannerConfig.default_config = read_config("/opt/tanner/config.yaml") + TannerConfig.default_config = read_config(default_config_path) @staticmethod def set_config(config_path): diff --git a/tanner/config.yaml b/tanner/data/config.yaml similarity index 100% rename from tanner/config.yaml rename to tanner/data/config.yaml * Use global variable instead of Meta class * Make read_config function a static method under ReadConfig class * Move read_config function under TannerConfig class * removed get_section() and used get() in its place Also added a check for old config format * Update config docs * catch exception while reading the config file. Also added some comment for config docs. And removed a comma from a config

Use YAML format for tanner configuration

b431a1d

mzfr force-pushed the yaml-config branch from 4b466df to b431a1d Compare March 9, 2020 14:41

mzfr added 3 commits March 9, 2020 20:11

Add pyyaml to requirements.txt

cb9d3fa

remove usage of self in TannerConfig class

da5094a

Since the old config was using staticmethod decorator I decided to use those

Fix the format of YAML config

dc3b4b0

mzfr force-pushed the yaml-config branch from c987bc0 to dc3b4b0 Compare March 9, 2020 16:15

mzfr added 2 commits March 9, 2020 23:15

Use metaclass to setup default values

98c64f7

Update config test

f8543b5

mzfr force-pushed the yaml-config branch from 5cb5f83 to d68fcf4 Compare March 9, 2020 18:22

mzfr force-pushed the yaml-config branch from d68fcf4 to 0010caf Compare March 9, 2020 18:29

afeena reviewed Mar 9, 2020

View reviewed changes

Use global variable instead of Meta class

5a3542d

afeena reviewed Mar 10, 2020

View reviewed changes

Make read_config function a static method under ReadConfig class

6b92445

afeena reviewed Mar 11, 2020

View reviewed changes

Move read_config function under TannerConfig class

9c8745c

mzfr added 2 commits March 13, 2020 22:56

removed get_section() and used get() in its place

f37f4f6

Also added a check for old config format

Update config docs

afb442d

afeena reviewed Mar 16, 2020

View reviewed changes

catch exception while reading the config file.

0dd0fdb

Also added some comment for config docs. And removed a comma from a config

mzfr force-pushed the yaml-config branch from cc3d0ea to 0dd0fdb Compare March 17, 2020 06:43

mzfr changed the base branch from master to develop March 18, 2020 15:37

afeena merged commit 0793c94 into mushorg:develop Mar 18, 2020

afeena mentioned this pull request Mar 20, 2020

Prepare docker for using yml configs #380

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use YAML format for tanner configuration #371

Use YAML format for tanner configuration #371

mzfr commented Mar 9, 2020 •

edited

Loading

coveralls commented Mar 9, 2020 •

edited

Loading

mzfr commented Mar 9, 2020

afeena Mar 9, 2020

mzfr Mar 10, 2020

afeena Mar 9, 2020 •

edited

Loading

mzfr Mar 10, 2020

afeena Mar 10, 2020

afeena Mar 10, 2020

afeena Mar 10, 2020

afeena commented Mar 10, 2020

mzfr commented Mar 11, 2020

afeena commented Mar 11, 2020

afeena Mar 11, 2020

Parth1811 commented Mar 11, 2020

mzfr commented Mar 12, 2020

afeena commented Mar 12, 2020 •

edited

Loading

mzfr commented Mar 13, 2020

afeena Mar 16, 2020 •

edited

Loading

afeena Mar 16, 2020

afeena commented Mar 17, 2020

mzfr commented Mar 17, 2020

afeena commented Mar 18, 2020

mzfr commented Mar 18, 2020

afeena commented Mar 18, 2020

		return config_values


		DEFAULT_CONFIG = read_config("/opt/tanner/data/config.yaml")

Use YAML format for tanner configuration #371

Use YAML format for tanner configuration #371

Conversation

mzfr commented Mar 9, 2020 • edited Loading

coveralls commented Mar 9, 2020 • edited Loading

Pull Request Test Coverage Report for Build 1233

💛 - Coveralls

mzfr commented Mar 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afeena Mar 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afeena commented Mar 10, 2020

mzfr commented Mar 11, 2020

afeena commented Mar 11, 2020

Choose a reason for hiding this comment

Parth1811 commented Mar 11, 2020

mzfr commented Mar 12, 2020

afeena commented Mar 12, 2020 • edited Loading

mzfr commented Mar 13, 2020

afeena Mar 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afeena commented Mar 17, 2020

mzfr commented Mar 17, 2020

afeena commented Mar 18, 2020

mzfr commented Mar 18, 2020

afeena commented Mar 18, 2020

mzfr commented Mar 9, 2020 •

edited

Loading

coveralls commented Mar 9, 2020 •

edited

Loading

afeena Mar 9, 2020 •

edited

Loading

afeena commented Mar 12, 2020 •

edited

Loading

afeena Mar 16, 2020 •

edited

Loading