Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate Token Auth in Notebooks #141

Merged
merged 5 commits into from
Jul 8, 2024
Merged

Integrate Token Auth in Notebooks #141

merged 5 commits into from
Jul 8, 2024

Conversation

neshdev
Copy link
Member

@neshdev neshdev commented Jul 3, 2024

Token Auth will implicitly grant permissions to kaggle resources (models and datasets)

  • Added unit test using flask-jwt package to simulate oauth tokens.
  • Notebook Testing in b/351825012
  • Infra Token implementation in b/343728546

@neshdev neshdev requested review from a team, psbang and rosbo July 8, 2024 16:46
@@ -171,9 +174,11 @@ def download_file(self, path: str, out_file: str, resource_handle: Optional[Reso
_CHECKSUM_MISMATCH_MSG_TEMPLATE.format(expected_md5_hash, actual_md5_hash)
)

def _get_http_basic_auth(self) -> Optional[HTTPBasicAuth]:
def _get_auth(self) -> Optional[requests.auth.AuthBase]:
Copy link
Contributor

Choose a reason for hiding this comment

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

I like to refactor to rely on AuthBase and have KaggleTokenAuth extend it. Very clean 👏

@rosbo
Copy link
Contributor

rosbo commented Jul 8, 2024

Did you also test it e2e in a Kaggle notebook?

You can run the following in a cell (you probably already know, but sharing just in case):

!pip install --force-reinstall --no-deps git+https://github.com/Kaggle/kagglehub.git@neshdev-v2-auth

@neshdev neshdev merged commit f680258 into main Jul 8, 2024
7 checks passed
@neshdev neshdev deleted the neshdev-v2-auth branch July 8, 2024 18:26
@neshdev
Copy link
Member Author

neshdev commented Jul 8, 2024

Did you also test it e2e in a Kaggle notebook?

You can run the following in a cell (you probably already know, but sharing just in case):

!pip install --force-reinstall --no-deps git+https://github.com/Kaggle/kagglehub.git@neshdev-v2-auth

Yes. I did e2e testing here. Added my notes here:
https://b.corp.google.com/issues/351825012

It worked as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants