Convert arg strings to enum values #644
Conversation
kaggle/api/kaggle_api_extended.py
Outdated
| @@ -298,7 +306,7 @@ class KaggleApi(KaggleApi): | |||
| config = os.path.join(config_dir, config_file) | |||
| config_values = {} | |||
| already_printed_version_warning = False | |||
| args = {} # DEBUG Add --local to use localhost | |||
| args = {'--local'} # DEBUG Add --local to use localhost | |||
There was a problem hiding this comment.
It's a bit tedious to keep changing it during development. DEBUG is like TODO in that it won't be in the release. I'd prefer to leave it like this for now and change it prior to release.
There was a problem hiding this comment.
Can you clarify? Is there some automated tool that strips this out? I'm fine with it if it's automated, but not if it's a manual step that someone has to do prior to release, because e.g. someone else might do the release and not know that this needs to be done.
There was a problem hiding this comment.
Generally it is LGTM, but I can't approve unreleasable and/or untested changes into main. While we don't anticipate anything, priorities and ownerships can change quickly, and work that an engineer planned to do "real soon" gets delayed. It might be worth a quick sync to figure out a strategy that lets you keep moving quickly while preserving our ability to release.
kaggle/api/kaggle_api_extended.py
Outdated
| @@ -298,7 +306,7 @@ class KaggleApi(KaggleApi): | |||
| config = os.path.join(config_dir, config_file) | |||
| config_values = {} | |||
| already_printed_version_warning = False | |||
| args = {} # DEBUG Add --local to use localhost | |||
| args = {'--local'} # DEBUG Add --local to use localhost | |||
There was a problem hiding this comment.
Can you clarify? Is there some automated tool that strips this out? I'm fine with it if it's automated, but not if it's a manual step that someone has to do prior to release, because e.g. someone else might do the release and not know that this needs to be done.
tests/unit_tests.py
Outdated
| 0) # Assuming there should be some competitions | ||
| competitions = api.competitions_list(group='general') | ||
| self.assertGreater(len(competitions), 0) | ||
| self.assertLess(len(competitions), 20) |
There was a problem hiding this comment.
nit - I think this should be assertLessEqual? Assuming the page size is 20, it should be possible in the future to get 20, depending on how many are active.
|
Potential release blockers have been deleted. PTAL @jplotts and thanks for the review. |
Fixes #641 as a side effect.