Skip to content

Conversation

@BinoyOza-okta
Copy link
Contributor

@BinoyOza-okta BinoyOza-okta commented Jan 7, 2026

@BinoyOza-okta BinoyOza-okta changed the title Clear cache - Implementation https://github.com/okta/okta-jwt-verifier-python/pull/45 Clear cache if http client fails Jan 7, 2026
@BinoyOza-okta BinoyOza-okta force-pushed the OKTA-1061735 branch 4 times, most recently from c170837 to be9c0fa Compare January 7, 2026 20:13
@aniket-okta
Copy link

Please remove the debug print statement on line 217:

print('String e: '+ str(e))

This should either be removed entirely or replaced with proper logging using Python's logging module. Also consider re-raising the exception after clearing the cache so callers are aware of the failure.

aniket-okta
aniket-okta previously approved these changes Jan 8, 2026
Copy link

@aniket-okta aniket-okta left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@BinoyOza-okta
Copy link
Contributor Author

Please remove the debug print statement on line 217:

print('String e: '+ str(e))

This should either be removed entirely or replaced with proper logging using Python's logging module. Also consider re-raising the exception after clearing the cache so callers are aware of the failure.

Regarding the print statement: You are right, that was left over from my debugging session where standard logging wasn't capturing the output. I will remove it to keep the library output clean.

Regarding re-raising the exception: I would advise against re-raising here. The critical issue this PR fixes is that a single failure causes the library to hang indefinitely on subsequent calls because the cache holds onto a failed future.

The goal of this block is cleanup: we catch the failure, remove the bad entry from the cache, and allow the program to continue. If we re-raise, we stop the execution flow, which defeats the purpose of allowing the application to recover and retry on the next tick. I want to ensure the library handles the network disconnect gracefully rather than crashing the process."

@BinoyOza-okta BinoyOza-okta merged commit 7d565fb into master Jan 8, 2026
3 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