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

In Cloud, out of date connectors call always returns 0 #21126

Merged

Conversation

krishnaglick
Copy link
Contributor

What

In Cloud, out of date connectors call always returns 0

How

The app checks if it's running in Cloud mode, and if so doesn't make the call to the server to get out of date connectors; it just returns 0 out of date connectors.

Additionally, the check now properly returns a boolean.

@krishnaglick krishnaglick requested a review from a team as a code owner January 6, 2023 19:27
@octavia-squidington-iv octavia-squidington-iv added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Jan 6, 2023
@krishnaglick krishnaglick enabled auto-merge (squash) January 6, 2023 19:27
Copy link
Contributor

@lmossman lmossman left a comment

Choose a reason for hiding this comment

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

Changes LGTM

@krishnaglick krishnaglick merged commit c1cb763 into master Jan 6, 2023
@krishnaglick krishnaglick deleted the kc/cloud-out-of-date-connectors-call-always-returns-0 branch January 6, 2023 20:11
import { AirbyteRequestService } from "core/request/AirbyteRequestService";
import { useDefaultRequestMiddlewares } from "services/useDefaultRequestMiddlewares";
import { useInitService } from "services/useInitService";
import { isCloudApp } from "utils/app";
Copy link
Contributor

Choose a reason for hiding this comment

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

This is great as a hotfix—small, simple, direct, effective—but I do think that a good follow-up would be editing the conditional to check if FeatureItem.AllowUpdateConnectors is enabled instead of checking isCloud(). It looks like we don't really have an API for that besides useFeature, which is a bummer: right now this module doesn't introduce any react-specific context like hooks impose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doing conditionals with hooks always gets awkward, but yeah.
We're going to revert the backend change Monday so maybe @josephkmh can make that change if he gets to it before I'm online!

Copy link
Contributor

@josephkmh josephkmh Jan 6, 2023

Choose a reason for hiding this comment

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

@ambirdsall agreed - I have an open issue here proposing that approach. I'll keep that issue open as a refactor of this hotfix.

jbfbell pushed a commit that referenced this pull request Jan 13, 2023
* isCloudApp returns a boolean now

* Connector service never makes the check updates call in Cloud
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants