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

Add /v1/web_backend/check_updates #20041

Merged
merged 7 commits into from
Dec 8, 2022

Conversation

gosusnp
Copy link
Contributor

@gosusnp gosusnp commented Dec 3, 2022

What

  • Add a new endpoint to retrieve the number of updatable connectors.

Relates to #19853

How

  • Add new endpoint.

@gosusnp gosusnp requested a review from krishnaglick December 3, 2022 01:58
@octavia-squidington-iv octavia-squidington-iv added area/api Related to the api area/documentation Improvements or additions to documentation area/platform issues related to the platform area/server labels Dec 3, 2022
@gosusnp gosusnp temporarily deployed to more-secrets December 3, 2022 01:59 Inactive
.toList();
} catch (final IOException e) {
log.error("Failed to get current list of standard destination definitions", e);
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: consider extracting to a constant to avoid re-use/magic number.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@jdpgrailsdev jdpgrailsdev left a comment

Choose a reason for hiding this comment

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

:shipit: with one small nit.

@gosusnp gosusnp temporarily deployed to more-secrets December 5, 2022 19:26 Inactive
Copy link
Contributor

@benmoriceau benmoriceau left a comment

Choose a reason for hiding this comment

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

2 nit comments

final static boolean INCLUDE_TOMBSTONE = false;

@BeforeEach
void beforeEach() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this could be replace by the annotation @mock and @InjectMock

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Played with the annotations, I think a setup function feels clearer than having annotations on both members + class to figure out how the objects are initialized.
Might be related to the fact that I am not a big fan of the lombok type of annotations.

@gosusnp gosusnp temporarily deployed to more-secrets December 5, 2022 20:43 Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 5, 2022 21:54 Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 5, 2022 21:57 Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 5, 2022 23:48 Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 5, 2022 23:49 Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 5, 2022 23:49 Inactive
Copy link
Contributor

@krishnaglick krishnaglick left a comment

Choose a reason for hiding this comment

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

I did a test implementation on this branch and it's looking good! Seems to correctly update the count when the user upgrades 👍

@gosusnp gosusnp temporarily deployed to more-secrets December 7, 2022 03:04 — with GitHub Actions Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 7, 2022 03:04 — with GitHub Actions Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 7, 2022 13:43 — with GitHub Actions Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 7, 2022 13:43 — with GitHub Actions Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 7, 2022 16:40 — with GitHub Actions Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 7, 2022 16:41 — with GitHub Actions Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 7, 2022 18:32 — with GitHub Actions Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 7, 2022 18:33 — with GitHub Actions Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 7, 2022 22:27 — with GitHub Actions Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 7, 2022 22:27 — with GitHub Actions Inactive
@gosusnp gosusnp temporarily deployed to more-secrets December 7, 2022 23:48 — with GitHub Actions Inactive
@gosusnp gosusnp merged commit 315c152 into master Dec 8, 2022
@gosusnp gosusnp deleted the gosusnp/19853-out-of-date-definitions-endpoint branch December 8, 2022 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Related to the api area/documentation Improvements or additions to documentation area/platform issues related to the platform area/server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants