Skip to content

[FEATURE] Add better context to DataApiExceptionss#16

Merged
shtrom merged 1 commit into
masterfrom
fix/dataapiexception-better-context
May 13, 2019
Merged

[FEATURE] Add better context to DataApiExceptionss#16
shtrom merged 1 commit into
masterfrom
fix/dataapiexception-better-context

Conversation

@shtrom

@shtrom shtrom commented Mar 29, 2019

Copy link
Copy Markdown
Contributor

Signed-off-by: Olivier Mehani olivier.mehani@learnosity.com

Checklist

  • Feature

  • Bug

  • Security

  • Documentation

  • ChangeLog.md updated

  • Tests added

  • All testsuites passed

  • make dist completed successfully

Signed-off-by: Olivier Mehani <olivier.mehani@learnosity.com>
@shtrom shtrom force-pushed the fix/dataapiexception-better-context branch from ccba066 to 17f62b2 Compare March 29, 2019 04:02
@shtrom shtrom changed the title [FEATURE] Add better context to s [FEATURE] Add better context to DataApiExceptionss Mar 29, 2019
@shtrom

shtrom commented May 13, 2019

Copy link
Copy Markdown
Contributor Author

@eoghanmcilwaine tit for tat: can you review this one for me? 😁

@shtrom shtrom requested a review from eoghanmcilwaine May 13, 2019 05:22

@eoghanmcilwaine eoghanmcilwaine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good @shtrom. As a future improvement it might be worth catching potential exceptions from requests to give more specific feedback: https://2.python-requests.org//en/latest/user/quickstart/#errors-and-exceptions

@shtrom

shtrom commented May 13, 2019

Copy link
Copy Markdown
Contributor Author

Yup, it would be nice to expose the underlying Exception.

@shtrom

shtrom commented May 13, 2019

Copy link
Copy Markdown
Contributor Author

Thanks for the review!

@shtrom shtrom merged commit d56f7f3 into master May 13, 2019
@shtrom shtrom deleted the fix/dataapiexception-better-context branch May 13, 2019 07:50
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