Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Expand simple logging options #4832

Merged
merged 14 commits into from
Aug 28, 2016
Merged

Expand simple logging options #4832

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
794a959
Fix bot crash at start on permaban
Gobberwart Aug 27, 2016
41403d3
Merge branch 'PokemonGoF/dev' into dev
Gobberwart Aug 27, 2016
e7ec70e
Merge branch 'PokemonGoF/dev' into dev
Gobberwart Aug 27, 2016
8df9aa7
Merge branch 'PokemonGoF/dev' into dev
Gobberwart Aug 27, 2016
041e5ff
Expanded logging options
Gobberwart Aug 27, 2016
bb844b9
Added warning about deprecated logging_color arg
Gobberwart Aug 27, 2016
0cf9655
Display log message moved
Gobberwart Aug 27, 2016
f88997d
Remove milliseconds from datetime
Gobberwart Aug 27, 2016
79708f2
Merge branch 'PokemonGoF/dev' into dev
Gobberwart Aug 28, 2016
bbe0dfe
Merge branch 'Pokemongof/dev' into dev
Gobberwart Aug 28, 2016
c9164a7
Reversed condition order for clarity
Gobberwart Aug 28, 2016
1ccde94
Documented new log options
Gobberwart Aug 28, 2016
33e1095
Modified conditions again
Gobberwart Aug 28, 2016
0624ba9
Merge branch 'pokemongof/dev' into dev
Gobberwart Aug 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions configs/config.json.cluster.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,12 @@
"location_cache": true,
"distance_unit": "km",
"reconnecting_timeout": 15,
"logging": {
"color": true,
"clean": false
},
"logging": {
"color": true,
"show_datetime": true,
"show_process_name": true,
"show_log_level": true
},
"catch": {
"any": {"catch_above_cp": 0, "catch_above_iv": 0, "logic": "or"},
"// Example of always catching Rattata:": {},
Expand Down
10 changes: 6 additions & 4 deletions configs/config.json.map.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,10 +485,12 @@
"location_cache": true,
"distance_unit": "km",
"reconnecting_timeout": 15,
"logging": {
"color": true,
"clean": false
},
"logging": {
"color": true,
"show_datetime": true,
"show_process_name": true,
"show_log_level": true
},
"catch": {
"any": {"catch_above_cp": 0, "catch_above_iv": 0, "logic": "or"},
"// Example of always catching Rattata:": {},
Expand Down
10 changes: 6 additions & 4 deletions configs/config.json.optimizer.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,12 @@
"location_cache": true,
"distance_unit": "km",
"reconnecting_timeout": 15,
"logging": {
"color": true,
"clean": false
},
"logging": {
"color": true,
"show_datetime": true,
"show_process_name": true,
"show_log_level": true
},
"catch": {
"any": {
"always_catch": true
Expand Down
10 changes: 6 additions & 4 deletions configs/config.json.path.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,10 +234,12 @@
"location_cache": true,
"distance_unit": "km",
"reconnecting_timeout": 15,
"logging": {
"color": true,
"clean": false
},
"logging": {
"color": true,
"show_datetime": true,
"show_process_name": true,
"show_log_level": true
},
"catch": {
"any": {"catch_above_cp": 0, "catch_above_iv": 0, "logic": "or"},
"// Example of always catching Rattata:": {},
Expand Down
10 changes: 6 additions & 4 deletions configs/config.json.pokemon.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,10 +238,12 @@
"location_cache": true,
"distance_unit": "km",
"reconnecting_timeout": 15,
"logging": {
"color": true,
"clean": false
},
"logging": {
"color": true,
"show_datetime": true,
"show_process_name": true,
"show_log_level": true
},
"catch": {
"any": {"catch_above_cp": 0, "catch_above_iv": 0, "logic": "or" },

Expand Down
8 changes: 6 additions & 2 deletions pokecli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ def _json_loader(filename):
type=bool,
default=False
)

# Start to parse other attrs
config = parser.parse_args()
if not config.username and 'username' not in load:
Expand All @@ -650,6 +650,7 @@ def _json_loader(filename):
config.live_config_update = load.get('live_config_update', {})
config.live_config_update_enabled = config.live_config_update.get('enabled', False)
config.live_config_update_tasks_only = config.live_config_update.get('tasks_only', False)
config.logging = load.get('logging', {})

if config.map_object_cache_time < 0.0:
parser.error("--map_object_cache_time is out of range! (should be >= 0.0)")
Expand Down Expand Up @@ -694,7 +695,10 @@ def task_configuration_error(flag_name):

if "daily_catch_limit" in load:
logger.warning('The daily_catch_limit argument has been moved into the CatchPokemon Task')


if "logging_color" in load:
logger.warning('The logging_color argument has been moved into the logging config section')

if config.walk_min < 1:
parser.error("--walk_min is out of range! (should be >= 1.0)")
return None
Expand Down
16 changes: 13 additions & 3 deletions pokemongo_bot/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def start(self):
def _setup_event_system(self):
handlers = []

if self.config.logging_color:
if self.config.logging and 'color' in self.config.logging and self.config.logging['color']:
handlers.append(ColoredLoggingHandler(self))
else:
handlers.append(LoggingHandler(self))
Expand Down Expand Up @@ -760,8 +760,18 @@ def _setup_logging(self):
logging.getLogger("pgoapi").setLevel(log_level)
logging.getLogger("rpc_api").setLevel(log_level)

if self.config.logging_clean and not self.config.debug:
formatter = Formatter(fmt='[%(asctime)s] %(message)s', datefmt='%H:%M:%S')
if self.config.logging:
logging_format = '%(message)s'
logging_format_options = ''
if ('show_log_level' in self.config.logging and self.config.logging['show_log_level']) or 'show_log_level' not in self.config.logging:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we check if 'show_log_level' is not present, and if so, set it to true. Then a second if for the actual format change?

Same with those below.

Copy link
Contributor Author

@Gobberwart Gobberwart Aug 28, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what it does. First check is for "is in config and set true", second check is OR "not in config". If either condition is matched, it will be enabled.

Copy link
Contributor

@mjmadsen mjmadsen Aug 28, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get that. I'm a bit curious on the evaluation order in python as the "and self.config.logging['show_log_level'] might happen before we check if it is in self.config.logging.

Suppose we could put 'show_log_level' in self.config.logging inside of more parenthesis to make sure it happens first. If you could try to run it (as is) without them being defined, to see if there is an error. I would do this quick, but I'm at work (and working really hard :P).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't happen... the first condition (with the end) will fail on the first check and not even bother to perform the second.

For the sake of clarity, I'll reverse it :)

Copy link
Contributor

@mjmadsen mjmadsen Aug 28, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's subtle.
('show_log_level' in self.config.logging and self.config.logging['show_log_level'])

Might evaluate self.config.logging['show_log_level'] first, which would throw an error if it doesn't exist.

How about this:
if ('show_log_level' not in self.config.logging) or self.config.logging['show_log_level']):

Copy link
Contributor Author

@Gobberwart Gobberwart Aug 28, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjmadsen How's this (in the latest commit for all three items)...?

            if 'show_log_level' not in self.config.logging or \
                    ('show_log_level' in self.config.logging and self.config.logging['show_log_level']):
                logging_format = '[%(levelname)s] ' + logging_format

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hahaha, I'm a derp. If the ('show_log_level' in self.config.logging) fails, I really don't need to check it again, do I?

2 minutes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, done and tested. All lines now follow the format you suggested as follows:

            if ('show_log_level' not in self.config.logging) or self.config.logging['show_log_level']:
                logging_format = '[%(levelname)s] ' + logging_format

I knew something wasn't quite right in there.

logging_format = '[%(levelname)s] ' + logging_format
if ('show_process_name' in self.config.logging and self.config.logging['show_process_name']) or 'show_process_name' not in self.config.logging:
logging_format = '[%(name)10s] ' + logging_format
if ('show_datetime' in self.config.logging and self.config.logging['show_datetime']) or 'show_datetime' not in self.config.logging:
logging_format = '[%(asctime)s] ' + logging_format
logging_format_options = '%Y-%m-%d %H:%M:%S'

formatter = Formatter(logging_format,logging_format_options)
for handler in logging.root.handlers[:]:
handler.setFormatter(formatter)

Expand Down