Bug/skip date with output #314
Conversation
- added skip as an option to the configuration - added the correct regex for file names to the example config and set it as a default value - removed unnecessary fields from the minimal config example
- added skip as an option to the configuration - added the correct regex for file names to the example config and set it as a default value - removed unnecessary fields from the minimal config example
…a/open-bus into bug/skip-date-with-output
| }, | ||
| "output_file_name_regexp": "^(?P<date_str>[^_]+?)_(?P<type>\\w+)", | ||
| "output_file_type": "csv.gz" | ||
| } |
There was a problem hiding this comment.
Why did you remove output_file_name_regexp? If you changed it to be optional, specify it in the schema
| @@ -7,7 +7,7 @@ | |||
| "filtered_feeds": "filtered_feeds", | |||
| "logs": "logs" | |||
| }, | |||
| "output_file_name_regexp": "^(?P<date_str>[^_]+?)_(?P<type>\\w+)", | |||
| "output_file_name_regexp": "^(?P<type>\\w+)_(?P<date_str>[^_]+?)", | |||
There was a problem hiding this comment.
Are you sure about this change? Did you consult with @cjer?
There was a problem hiding this comment.
As you can see here, there is no option to change the format of the filenames:
date_str = date.strftime('%Y-%m-%')
trip_stats_output_path = join(output_folder, f'trip_stats_{date_str}.{output_file_type}')
route_stats_output_path = join(output_folder, f'route_stats_{date_str}.{output_file_type}')
| @@ -28,6 +28,12 @@ | |||
| "type": "boolean", | |||
| "default": false | |||
| }, | |||
| "skip_date_with_output": { | |||
|
|
||
| # validate that the regex used the correct group names | ||
| faulty_group_names = False | ||
| if "type" in regexp.groupindex and "date_str" in regexp.groupindex: |
There was a problem hiding this comment.
Shouldn't the condition be negated?
| file_type = (parse_conf_date_format(date_str), stats_type) | ||
| file_type = _parse_file_name_regex_match(match, faulty_group_names) | ||
| if file_type is None: | ||
| # return empty list if there was an error in one of the files |
There was a problem hiding this comment.
I'd guess we'd like to return only the found files. Why would we give up on all of the files if one failed?
There was a problem hiding this comment.
I thought that it is indicating of something weird going on, but maybe you are right.
I can just log that and continue
|
|
||
| # validate that the regex used the correct group names | ||
| faulty_group_names = False | ||
| if "type" in regexp.groupindex and "date_str" in regexp.groupindex: |
There was a problem hiding this comment.
Wouldn't you like to put it inside _parse_file_name_regex_match?
There was a problem hiding this comment.
I thought about that but than the log message would log for every file, although they are all using the same regex.
I thought creating a global variable just for that is a bit disgusting.
|
Does anyone knows why github thinks the travis build are still running? |
Just a bug in GitHub, I guess. An administrator can merge even without the checks passing, so don't worry. |
|
I think we can merge. |
I reopened the PR because it was stuck in an odd state.
The changes are to the part that searches for existing output files.
--
From the description of the original PR (#313):