-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
🪟 🐛 Poll backend for Free Connector Program enrollment success #22289
🪟 🐛 Poll backend for Free Connector Program enrollment success #22289
Conversation
8320b37
to
14215ce
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.
Nice utility! I imagine this could be useful for short polling in other instances 👍
id: "fcp/enrolled", | ||
text: formatMessage({ id: "freeConnectorProgram.enroll.success" }), | ||
type: ToastType.SUCCESS, | ||
pollUntil( |
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.
We have pretty similar code in
airbyte/airbyte-webapp/src/packages/cloud/views/credits/CreditsPage/components/RemainingCredits.tsx
Lines 73 to 91 in 6a74106
useEffectOnce(() => { | |
// If we are coming back from a successfull stripe checkout ... | |
if (searchParams.has(STRIPE_SUCCESS_QUERY)) { | |
// Remove the stripe parameter from the URL | |
setSearchParams({}, { replace: true }); | |
// If the workspace doesn't have a recent increase in credits our server has not yet | |
// received the Stripe callback or updated the workspace information. We're going to | |
// switch into a loading mode and relaod the workspace every 3s from now on until | |
// the workspace has received the credit update (see useEffect below) | |
if (!hasRecentCreditIncrease(cloudWorkspace)) { | |
setIsWaitingForCredits(true); | |
retryIntervalId.current = window.setInterval(() => { | |
invalidateCloudWorkspace(); | |
}, 3000); | |
} | |
} | |
return () => clearInterval(retryIntervalId.current); | |
}); |
pollUntil
utility.
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.
Great callout. We also do a bunch of manual interval wrangling for the health check endpoint, too; it would be good to consolidate our polling approaches.
That said, it would be nice to get this change merged to close an annoying edge case first, so I'm inclined to ship those updates as a follow up PR.
5529451
to
694dce1
Compare
694dce1
to
db88c72
Compare
…tehq#22289) * Add pollUntil utility for polling Promises * poll backend for confirmed enrollment before showing success toast * Put interval and maxTimeout inside options arg * Give units w/ polling options: intervalMs and maxTimeoutMs
What
Closes #22302.
useFreeConnectorProgram()
with thefcpEnrollmentSuccess
query param, starts polling the FCP program info endpointHow
Adds a general-purpose
pollUntil
function, then uses that to poll the endpoint.Recommended reading order
useFreeConnectorProgram.ts
pollUntil.ts
orpollUntil.test.ts
, depending on how you feel about rxjs, followed by the other