Add option to add specific center or doctor#83
Add option to add specific center or doctor#83Dreiundzwanzig wants to merge 3 commits intorbignon:masterfrom
Conversation
Dreiundzwanzig
force-pushed
the
feature/add_specific_center
branch
from
b370403 to
5d4c2ae
Compare
Dreiundzwanzig
changed the title
Dreiundzwanzig
force-pushed
the
feature/add_specific_center
branch
from
5d4c2ae to
b684ab4
Compare
Co-authored-by: gitolicious <26963495+gitolicious@users.noreply.github.com>
|
Hello, Thank you for your patch. However, I think it is not user friendly to ask user providing the name and city, as the url is enough to guess them. In my opinion, the code may be a little refactored to not apply args filters (--center, --center-regex, etc.) on ones provided with --aditional-center. Another question related to the usecase itself, do you really want to add these centers to the search results, or are you targeting specifically these centers? Perhaps instead of --additional-center, it would be --center-url and in this case, do not do any search at all? |
|
Hello, i've added them to the search result, because at the time of writing the code i didn't want to introduce as as little as possible merge conflicts, but thats not neccesary anymore if it ever was. My personal usecase was to add additional centers/doctors from which i knew there offering vaccinations, but are not included in the search results and i wanted them to be processed additionally to the search results. You are right, i've refactored the code and removed requirement for name and city. |
| action='append', help='exclude centers') | ||
| parser.add_argument('--center-exclude-regex', | ||
| action='append', help='exclude centers by regex') | ||
| parser.add_argument('--additional-center', '-ac', |
There was a problem hiding this comment.
Until now, we only had single-letter abbreviations for arguments but it looks like we are running out of usable letters. Still it should be a deliberate decision to extend to two-letter abbreviations. @rbignon what do you think?
| logging.root.setLevel(level) | ||
| logging.root.addHandler(self.create_default_logger()) | ||
|
|
||
| def try_to_book_or_sleep(self, docto, center, vaccine_list, start_date, end_date, only_second, only_third, dry_run=False): |
There was a problem hiding this comment.
To me the method name is a bit misleading. "or sleep" in fact describes the internal logic of a lazy loading try_to_book || sleep but from the sound of it I would expect "or sleep" is related to the return value. Maybe someone comes up with a better name?
There was a problem hiding this comment.
Yeah your right, i guess we could just use try_to_book? I mean it's just that and in fact it does not matter if there is a sleep inside or not. What do you think?
4 participants
Add options to:
Closes #68, closes #82