Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 37 additions & 0 deletions .github/workflows/presubmit.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
name: presubmit

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All our CI/CD stuff is setup to use Google Cloud Build. We should stick with that rather than using GitHub workflow.

Can you add simply add a new "test step" after the existing step we already run on every PR:

- name: us-docker.pkg.dev/$PROJECT_ID/tools/cli-builder:${_PYTHON_VERSION}
id: lint
args:
- run
- lint:all
waitFor: ['build-hatch-image']

on:
pull_request:
push:
branches:
- main
workflow_dispatch:

permissions:
contents: read

jobs:
checker:
runs-on: ubuntu-latest
strategy:
fail-fast: true
matrix:
os: [ubuntu-latest]
python-version: ['3.11']
steps:
- name: checkout
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install .
pip install pytest

- name: Run tests
run: |
tests/run_tests.sh
71 changes: 29 additions & 42 deletions src/kaggle/api/kaggle_api_extended.py
Original file line number Diff line number Diff line change
Expand Up @@ -972,12 +972,18 @@ def retriable_func(*args):

## Authentication

def _load_config(self) -> None:
"""Load configuration from file and environment variables."""
config_values = self.read_config_file(quiet=True)
self.config_values = self.read_config_environment(config_values)

def authenticate(self) -> None:
"""Authenticate the user with the Kaggle API, using either a legacy API key or a Kaggle OAuth token.

Returns:
None:
"""
self._load_config()
if self._authenticate_with_access_token():
return
if self._authenticate_with_legacy_apikey():
Expand All @@ -997,36 +1003,20 @@ def _authenticate_with_legacy_apikey(self) -> bool:
Returns:
bool: True if auth succeeded.
"""

config_values: Dict[str, str] = {}

# Ex: 'datasets list', 'competitions files', 'models instances get', etc.
api_command = " ".join(sys.argv[1:])

# Step 1: try getting username/password from environment
config_values = self.read_config_environment(config_values)
if self.CONFIG_NAME_USER in self.config_values and self.CONFIG_NAME_KEY in self.config_values:
self.config_values[self.CONFIG_NAME_AUTH_METHOD] = str(AuthMethod.LEGACY_API_KEY)
self.logger.debug(f"Authenticated with legacy api key in: {self.config}")
return True

# Step 2: if credentials were not in env read in configuration file
if self.CONFIG_NAME_USER not in config_values or self.CONFIG_NAME_KEY not in config_values:
if os.path.exists(self.config):
config_values = self.read_config_file(config_values, quiet=True)
elif self._command_allows_logged_out(api_command):
config_values = self.read_config_file(config_values, quiet=True)
return True
else:
return False
if self._command_allows_logged_out(api_command):
return True

# Step 3: Validate and save
# Username and password are required.
for item in [self.CONFIG_NAME_USER, self.CONFIG_NAME_KEY]:
if item not in config_values:
raise ValueError("Error: Missing %s in configuration." % item)
self.config_values = config_values
self.config_values[self.CONFIG_NAME_AUTH_METHOD] = str(AuthMethod.LEGACY_API_KEY)
self.logger.debug(f"Authenticated with legacy api key in: {self.config}")
return True
return False

def _authenticate_with_access_token(self):
def _authenticate_with_access_token(self) -> bool:
access_token, source = get_access_token_from_env()
if not access_token:
return False
Expand All @@ -1036,11 +1026,9 @@ def _authenticate_with_access_token(self):
self.logger.debug(f'Ignoring invalid/expired access token in "{source}".')
return False

self.config_values: Dict[str, str] = {
self.CONFIG_NAME_TOKEN: access_token,
self.CONFIG_NAME_USER: username,
self.CONFIG_NAME_AUTH_METHOD: str(AuthMethod.ACCESS_TOKEN),
}
self.config_values[self.CONFIG_NAME_TOKEN] = access_token
self.config_values[self.CONFIG_NAME_USER] = username
self.config_values[self.CONFIG_NAME_AUTH_METHOD] = str(AuthMethod.ACCESS_TOKEN)
self.logger.debug(f"Authenticated with access token in: {source}")
return True

Expand All @@ -1057,11 +1045,9 @@ def _authenticate_with_oauth_creds(self) -> bool:
creds.delete()
return False
raise
self.config_values: Dict[str, str] = {
self.CONFIG_NAME_TOKEN: access_token,
self.CONFIG_NAME_USER: creds.get_username(),
self.CONFIG_NAME_AUTH_METHOD: str(AuthMethod.OAUTH),
}
self.config_values[self.CONFIG_NAME_TOKEN] = access_token
self.config_values[self.CONFIG_NAME_USER] = creds.get_username()
self.config_values[self.CONFIG_NAME_AUTH_METHOD] = str(AuthMethod.OAUTH)
creds_path = os.path.expanduser(KaggleCredentials.DEFAULT_CREDENTIALS_FILE)
self.logger.debug(f"Authenticated with OAuth credentials in: {creds_path}")
return True
Expand Down Expand Up @@ -2242,22 +2228,23 @@ def competition_list_topics_cli(
if competition is None:
raise ValueError("No competition specified")

response = self.forums_list_topics(
forum_slug=competition,
if not quiet and (page_size is not None or page_token is not None or search is not None):
print(
"Warning: --page-size, --page-token, and --search are not supported for competition topics and will be ignored."
)

response = self.competition_list_topics(
competition=competition,
sort_by=sort_by,
page_size=page_size,
page_token=page_token,
search=search,
page=page,
)
topics = response.topics
if topics:
fields = self.forum_topic_fields
fields = self.competition_topic_fields
if csv_display:
self.print_csv(topics, fields)
else:
self.print_table(topics, fields)
if not quiet and response.next_page_token:
print(f"Next page token: {response.next_page_token}")
else:
print("No topics found")

Expand Down
16 changes: 11 additions & 5 deletions src/kaggle/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,14 @@ def _get_shared_topics_parser() -> argparse.ArgumentParser:
return shared


def _get_shared_competition_topics_parser() -> argparse.ArgumentParser:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you make sure your PR only includes related changes. i.e. the changes to setup the test runner.

shared = argparse.ArgumentParser(add_help=False)
shared.add_argument("-p", "--page", dest="page", type=int, default=1, required=False, help=Help.param_page)
shared.add_argument("-v", "--csv", dest="csv_display", action="store_true", help=Help.param_csv)
shared.add_argument("-q", "--quiet", dest="quiet", action="store_true", help=Help.param_quiet)
return shared


def parse_competitions(subparsers) -> None:
parser_competitions = subparsers.add_parser(
"competitions", formatter_class=argparse.RawTextHelpFormatter, help=Help.group_competitions, aliases=["c"]
Expand Down Expand Up @@ -380,13 +388,14 @@ def parse_competitions(subparsers) -> None:
parser_competitions_pages.set_defaults(func=api.competition_list_pages_cli)

shared_topics = _get_shared_topics_parser()
shared_competition_topics = _get_shared_competition_topics_parser()

# Competitions list discussion topics (with 'show' and 'list' subcommands)
parser_competitions_topics = subparsers_competitions.add_parser(
"topics",
formatter_class=argparse.RawTextHelpFormatter,
help=Help.command_competitions_topics,
parents=[shared_topics],
parents=[shared_competition_topics],
)
subparsers_competitions_topics = parser_competitions_topics.add_subparsers(title="commands", dest="command")
subparsers_competitions_topics.choices = Help.entity_topics_choices
Expand All @@ -399,7 +408,7 @@ def parse_competitions(subparsers) -> None:
"list",
formatter_class=argparse.RawTextHelpFormatter,
help=Help.command_competitions_topics,
parents=[shared_topics],
parents=[shared_competition_topics],
)
parser_competitions_topics_list_optional = parser_competitions_topics_list._action_groups.pop()
parser_competitions_topics_list_optional.add_argument(
Expand All @@ -415,9 +424,6 @@ def parse_competitions(subparsers) -> None:
required=False,
help="Sort order. One of: " + ", ".join(KaggleApi.valid_forum_topic_sort_by),
)
parser_competitions_topics_list_optional.add_argument(
"--search", dest="search", required=False, help=Help.param_search
)
parser_competitions_topics_list._action_groups.append(parser_competitions_topics_list_optional)
parser_competitions_topics_list.set_defaults(func=api.competition_list_topics_cli)

Expand Down
32 changes: 32 additions & 0 deletions src/kaggle/test/test_authenticate.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import os
import unittest
from unittest.mock import patch


class TestAuthenticate(unittest.TestCase):
Expand Down Expand Up @@ -38,6 +39,37 @@ def test_config_actions(self):
self.assertTrue(api.config_dir.endswith("kaggle"))
self.assertEqual(api.get_config_value("doesntexist"), None)

@patch("kaggle.api.kaggle_api_extended.KaggleApi.read_config_file")
@patch("kaggle.api.kaggle_api_extended.KaggleApi._authenticate_with_oauth_creds")
def test_oauth_fallback_when_legacy_config_has_no_credentials(self, mock_oauth, mock_read_config):
username_env = os.environ.pop("KAGGLE_USERNAME", None)
key_env = os.environ.pop("KAGGLE_KEY", None)

try:
api = KaggleApi()
mock_read_config.return_value = {"proxy": "http://myproxy"}

def fake_oauth():
api.config_values["token"] = "oauth_token"
api.config_values["username"] = "oauth_user"
api.config_values["auth_method"] = "oauth"
return True

mock_oauth.side_effect = fake_oauth

api.authenticate()

self.assertEqual(api.config_values["token"], "oauth_token")
self.assertEqual(api.config_values["username"], "oauth_user")
self.assertEqual(api.config_values["auth_method"], "oauth")
self.assertEqual(api.config_values.get("proxy"), "http://myproxy")

finally:
if username_env is not None:
os.environ["KAGGLE_USERNAME"] = username_env
if key_env is not None:
os.environ["KAGGLE_KEY"] = key_env


if __name__ == "__main__":
unittest.main()
8 changes: 6 additions & 2 deletions src/kaggle/test/test_discussions_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

from kaggle.api.kaggle_api_extended import KaggleApi


# ---- Fixtures ----


Expand Down Expand Up @@ -119,15 +118,20 @@ def test_forums_topics_defaults_to_list(self, parser):
("datasets", ["list", "--sort-by", "hot", "-s", "keyword"]),
("models", ["--page-size", "50"]),
("benchmarks", ["--page-token", "abc"]),
("competitions", ["list", "--page", "5", "--sort-by", "recent"]),
],
ids=["datasets_sort_search", "models_page_size", "benchmarks_page_token"],
ids=["datasets_sort_search", "models_page_size", "benchmarks_page_token", "competitions_page_sort"],
)
def test_topics_list_with_options(self, parser, entity, extra_args):
"""Optional flags are parsed without error for entity topics."""
func, kwargs = _dispatch(parser, [entity, "topics"] + extra_args)
# Should dispatch to the list func, not show.
assert "show" not in func.__name__

def test_competitions_topics_list_does_not_accept_search(self, parser):
with pytest.raises(SystemExit):
parser.parse_args(["competitions", "topics", "list", "--search", "query"])


# ============================================================
# Arg parsing: topics show — correct func + kwargs
Expand Down
19 changes: 19 additions & 0 deletions tests/run_tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#!/bin/bash

# Exit immediately if a command exits with a non-zero status.
set -e

# Ensure we are in the tests directory.
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
cd "$SCRIPT_DIR"

echo "Running fast unit tests with pytest..."
(cd "$SCRIPT_DIR/.." && pytest)

# Run live integration tests only if stdin is a TTY (invoked interactively)
if [ -t 0 ]; then
echo "Running live integration tests (unit_tests.py)..."
python3 -m unittest unit_tests.py
fi

echo "All tests passed!"
Loading
Loading