Add ability to read IDs from files #482
Conversation
|
Kind of, this makes the BDFR more efficient over large series of IDs. There's a limit to how many you can fit on the command line before the OS errors out, and then you have to repeat the setup over and over again. That adds up over time. This makes it so that you can find IDs whichever way and then use it in a single command. Also makes it easier to use a variety of Bash tools and scripts like The one thing I think we shouldn't do is tie it to the GPDR templates. That's narrowing the focus of a broadly useful feature to a single use case when it should be trivial to extract the data from the CSV in the first place. I feel that the BDFR should be as data-agnostic as possible. It shouldn't matter where we got the list of IDs, just that there is a list. Then the BDFR can download them after whatever preprocessing the user desires has been done. |
|
@aliparlakci are we good to merge this PR? |
| @@ -17,6 +17,7 @@ | |||
| click.option('--authenticate', is_flag=True, default=None), | |||
| click.option('--config', type=str, default=None), | |||
| click.option('--disable-module', multiple=True, default=None, type=str), | |||
| click.option('--include-id-file', multiple=True, default=None), | |||
There was a problem hiding this comment.
--from-id-file more suitable
There was a problem hiding this comment.
I think the current name is better, it provides an opposite to the option --exclude-id-file. Since the latter already exists, I think the same convention for the former would be less confusing.
|
I have no other comment, it can be merged after the parameter name change. |
Implements #481 which people have been asking for for a while.