Skip to content

Conversation

@xgui3783
Copy link
Collaborator

@xgui3783 xgui3783 commented Dec 1, 2025

fixes #48

this is a quick fix. In future, we should introduce:

  • e2e test to fetch private bucket
  • e2e test to upload to bucket
  • (not sure possible) e2e test to fetch drive content
  • (not sure possible) e2e test to upload drive content

test: add test to prevent regression
@xgui3783 xgui3783 requested a review from apdavison December 1, 2025 08:34
Copy link
Member

@apdavison apdavison left a comment

Choose a reason for hiding this comment

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

I should note that we have end-to-end tests running on EBRAINS GitLab, but I'm fine with having read-only tests of public buckets also running on GitHub.



def wrap_exchange_seafile_token():
def exchnage_oidc_for_seafile(self: "DriveApiClient"):
Copy link
Member

Choose a reason for hiding this comment

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

should be "exchange" not "exchnage"

kwargs = copy(kwargs)

if self._seafile_token is None:
self._seafile_token = exchnage_oidc_for_seafile(self)
Copy link
Member

Choose a reason for hiding this comment

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

should be "exchange" not "exchnage"

return fn(self, *args, **kwargs)
except ClientHttpError as e:
if e.code == 401:
self._seafile_token = exchnage_oidc_for_seafile(self)
Copy link
Member

Choose a reason for hiding this comment

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

should be "exchange" not "exchnage"



_I_AM_A_PUBLIC_BUCKET = "_I_AM_A_PUBLIC_BUCKET"
_I_AM_A_PUBLIC_BUCKET = object()
Copy link
Member

Choose a reason for hiding this comment

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

can you explain the motivation for this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

mainly so that user cannot accidentally provide "_I_AM_A_PUBLIC_BUCKET" as a token, no matter now unlikely.

In thie case, even if the user provide object() as token kwarg, the is check will return false.

tests: add test on PR
@xgui3783 xgui3783 requested a review from apdavison December 1, 2025 14:37
@apdavison apdavison merged commit 7a6c383 into master Dec 1, 2025
9 checks passed
@xgui3783 xgui3783 deleted the fix/bucketClient branch December 1, 2025 20:02
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.

0.6.2 breaks data proxy client

3 participants