Skip to content

Conversation

ZXYmania
Copy link
Contributor

Add Examples for each of the exceptions

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • What is the current behavior? (You can also link to an open issue here)

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

@ZXYmania ZXYmania requested a review from a team as a code owner July 22, 2020 05:13
Copy link
Contributor

@mat-lord mat-lord left a comment

Choose a reason for hiding this comment

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

What are these examples for? Are they to display what exceptions can be raised? If so, we can do the following instead:

import staxapp.exceptions

for key, value in sorted(staxapp.exceptions.__dict__.items()):
    if isinstance(value, type):
        print(key)

Which returns this:

ApiException
InvalidCredentialsException
JSONDecodeError
ValidationException

@rowanu
Copy link

rowanu commented Jul 27, 2020

Can we please keep them as they are?

I like that it's clear an explicit as to the error types that are available, and that are being caught in each example.

except ApiException as e:
print(e)
if e.status_code == 404:
print("No accounts exist")
Copy link

Choose a reason for hiding this comment

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

Is "No accounts exist" the right text here?
Seems strange to get that back from a ReadCatalogueItems() call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've done this change

rowanu
rowanu previously approved these changes Aug 4, 2020
Copy link

@rowanu rowanu left a comment

Choose a reason for hiding this comment

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

🧮 💵

Add Examples for each of the exceptions
@codecov
Copy link

codecov bot commented Aug 4, 2020

Codecov Report

Merging #54 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master       #54   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          323       323           
=========================================
  Hits           323       323           

@Anton0 Anton0 added the documentation Improvements or additions to documentation label May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants