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

[v7] Fix exceptions not being raised together as CogniteMultiException #1456

Merged
merged 5 commits into from
Nov 1, 2023

Conversation

haakonvt
Copy link
Contributor

@haakonvt haakonvt commented Oct 30, 2023

Description

Fixes one of the oldest issues: #578

With this change, client.time_series.data.retrieve_latest(id=123), assuming this id doesn't randomly exist, now raises CogniteNotFoundError instead of CogniteAPIError.

@haakonvt haakonvt requested review from a team as code owners October 30, 2023 11:15
@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

❗ No coverage uploaded for pull request base (v7-release@c2de905). Click here to learn what that means.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             v7-release    #1456   +/-   ##
=============================================
  Coverage              ?   91.80%           
=============================================
  Files                 ?      121           
  Lines                 ?    15182           
  Branches              ?        0           
=============================================
  Hits                  ?    13938           
  Misses                ?     1244           
  Partials              ?        0           

@haakonvt haakonvt changed the title Fix exceptions not being raised as CogniteMultiException [v7] Fix exceptions not being raised together as CogniteMultiException Oct 30, 2023
Copy link
Contributor

@doctrino doctrino left a comment

Choose a reason for hiding this comment

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

Most of the work is already done by have the raise_compound_exception_if_failed_tasks?

@@ -236,7 +236,7 @@ def __str__(self) -> str:
return msg


class CogniteImportError(CogniteException):
class CogniteImportError(CogniteException, ImportError):
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice

@haakonvt
Copy link
Contributor Author

haakonvt commented Nov 1, 2023

Most of the work is already done by have the raise_compound_exception_if_failed_tasks?

Yes 🤩 I have no idea why someone started raising the first reception 🤷 My guess is that it was an error - and lots of people then copied it...

@haakonvt haakonvt merged commit 02f38e1 into v7-release Nov 1, 2023
5 of 7 checks passed
@haakonvt haakonvt deleted the fix-exceptions branch November 1, 2023 10:52
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