-
Notifications
You must be signed in to change notification settings - Fork 4.5k
🪟 🎉 Display service token validation errors in the UI #19578
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
Conversation
airbyte-webapp/src/packages/cloud/views/settings/integrations/DbtCloudSettingsView.tsx
Outdated
Show resolved
Hide resolved
airbyte-webapp/src/packages/cloud/views/settings/integrations/DbtCloudSettingsView.tsx
Outdated
Show resolved
Hide resolved
airbyte-webapp/src/packages/cloud/views/settings/integrations/DbtCloudSettingsView.tsx
Outdated
Show resolved
Hide resolved
onError: (e) => { | ||
setHasValidationError(true); | ||
|
||
setValidationMessage(e.message.replace("Internal Server Error: ", "")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the issue for the status code cleanup.
I also added a success message; to keep the review workflow as simple as possible for this PR, I put that commit into a separate PR. |
6a5c8a5
to
b66a72d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed it together over zoom, changes LGTM! Did not test locally
ee53f98
to
7ca93f9
Compare
All requested changes were addressed; dismissing so waking Tim up is not a blocker for merging before holiday code freeze
Also removes the "Error" from the `{setV,v}alidationErrorMessage` variables to pave the way for success feedback.
* Clean up type definitions Also removes the "Error" from the `{setV,v}alidationErrorMessage` variables to pave the way for success feedback. * Show success message and clear form on successful dbt cloud token submission * Always clear form error state and message in sync * Disallow deleting jobs while an update is pending
0963cf2
to
bab76fb
Compare
Note: the backend returns a 500 status (should be 400, or at least 4xx), so the frontend has to remove the error message's unsightly "Internal Server Error: " prefix in post. In the future, we want to move this to a 400 error.
* master: (74 commits) Fix support icon in sidebar fix: add BuildPulse report for helm ac tests (#19785) fix: yaml syntax (#19775) 🪟 🐛 Fix custom connection creation endpoint (#19702) Source facebook marketing: check "breakdowns" combinations (#19645) fix typo: notify instead of sync (#19737) find_non_rate_limited_PAT (#19736) Source Google Ads: fix schema for "campaigns" stream (#19700) 🎉 Source Asana: migrate to new SAT, added base HTTP errors handling (#19561) fix order not to randomly fail backward compatibility check (#19377) Bump helm chart version reference to 0.42.0 (#19706) fix: add extraEnv block (#19703) Bump Airbyte version from 0.40.21 to 0.40.22 (#19687) Bump helm chart version reference to 0.41.3 (#19685) Add connector builder server to airbyte proxy, kube overlays, and helm charts (#19554) dbt Cloud integration doc (#19619) 🪟 🎉 Display service token validation errors in the UI (#19578) 17644 Update Destination data type test to use the new data types (#19281) Docs: fix broken connector builder UI docs links (#19631) Bump Airbyte version from 0.40.20 to 0.40.21 (#19634) ...
What
Shows error state if backend's token validation fails.

Two issues:
I ran into a silly type error (default type forI figured out the issue; while I'm not sure why my valid call toonError
isunknown
, which didn't let me access its argument'smessage
property without a wholeas any
song-and-dance, including a linter magic comment 😢. There's a comment with a bit more detail.useMutation
wasn't typechecking, adding aMutationId
argument does, which lets me clean up the call site in the settings component."Internal Server Error: "
prefix in post (cc @mfsiega-airbyte)