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

Catch AssertionErrors and raise AnalyticsErrors instead #51

Merged
merged 4 commits into from
Apr 9, 2022

Conversation

PSalant726
Copy link
Contributor

Closes #49

SDK Changes

  • AnalyticsException --> AnalyticsError
    • Better aligns with Python convention
  • Add specific custom exceptions
    • Each inherits from AnalyticsError
  • Catch AssertionError when instantiating invalid AnalyticsClients and/or AnalyticsEvents, and raise useful AnalyticsErrors instead

Also renames `AnalyticsException` --> `AnalyticsError`, to align with
Python convention.

All error types inherit from `AnalyticsError` to enable consumers of
this library to simply catch `AnalyticsError`s in their code.
@PSalant726 PSalant726 added bug Something isn't working enhancement New feature or request labels Apr 7, 2022
@PSalant726 PSalant726 requested a review from SteveDMurphy April 7, 2022 22:26
@PSalant726 PSalant726 self-assigned this Apr 7, 2022
PSalant726 added a commit that referenced this pull request Apr 7, 2022
PSalant726 added a commit that referenced this pull request Apr 7, 2022
SteveDMurphy
SteveDMurphy previously approved these changes Apr 8, 2022
Copy link
Contributor

@SteveDMurphy SteveDMurphy left a comment

Choose a reason for hiding this comment

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

Looks great @PSalant726 !! Great changes all around and sorry for the holdup here.

Added a question around validating empty strings from events, but on further thought I am not so sure it should be something that kicks back an error 🤷🏽

fideslog/sdk/python/event.py Outdated Show resolved Hide resolved
@SteveDMurphy SteveDMurphy merged commit ad1a2eb into main Apr 9, 2022
@SteveDMurphy SteveDMurphy deleted the only-analytics-exceptions branch April 9, 2022 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Guarantee that all exceptions raised by the SDK are of type AnalyticsException
2 participants