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

tickets/DM-41550: Allow empty credentials/Workload Identity #120

Merged
merged 1 commit into from
Nov 22, 2023
Merged

tickets/DM-41550: Allow empty credentials/Workload Identity #120

merged 1 commit into from
Nov 22, 2023

Conversation

athornton
Copy link
Collaborator

If the account key is not set, fall back to workload identity rather than failing.

@athornton athornton marked this pull request as draft November 14, 2023 22:28
@athornton
Copy link
Collaborator Author

Because Giftless uses signed URLs, this is a bit more complex than the initial cut.

@athornton
Copy link
Collaborator Author

athornton commented Nov 18, 2023

It's working now (at least for me). Because the app is still pinned to Python 3.7, which is increasingly difficult to develop for (especially on an ARM Mac), I haven't even tried to update test cases, which would require updating test case cassettes, which drags me right into dependency hell.

The good news is, flask-classful finally got an update to 0.16 (and a new maintainer) so it might now be possible to rebuild the whole app with modern versions of packages and get to a current Python version. That will be a separate PR if my management approves my spending the effort to do so.

@athornton athornton marked this pull request as ready for review November 18, 2023 18:44
@athornton
Copy link
Collaborator Author

Addresses #108

@lappazos
Copy link

Fantastic, please merge @athornton

@rufuspollock
Copy link
Member

@athornton thank-you so much and we'd welcome upgrade to later python versions if you were up for it!.

LGTM and merging here.

@rufuspollock rufuspollock merged commit 903aacb into datopian:master Nov 22, 2023
0 of 4 checks passed
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.

3 participants