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

Set connector status to CONNECTED after successful validation and ping #3059

Merged

Conversation

artem-shelkovnikov
Copy link
Member

@artem-shelkovnikov artem-shelkovnikov commented Dec 25, 2024

Closes #3021

Closes https://github.com/elastic/search-team/issues/8535

Currently connectors have no way to report they're connected.

This PR makes it so that if a data source succeeds in validation and ping, then its status is set to CONNECTED.

In practice it means, that a user who had problems with their setup will see an error in UI. If they fix the credentials, before the error would go away after a sync starts. After this change the error will disappear within the service poll interval from user's action.

Checklists

Pre-Review Checklist

  • this PR does NOT contain credentials of any kind, such as API keys or username/passwords (double check config.yml.example)
  • this PR has a meaningful title
  • this PR links to all relevant github issues that it fixes or partially addresses
  • if there is no GH issue, please create it. Each PR should have a link to an issue
  • this PR has a thorough description
  • Covered the changes with automated tests
  • Tested the changes locally
  • Added a label for each target release version (example: v7.13.2, v7.14.0, v8.0.0)

Release Note

Fix a UI issue with connector reporting problems with connection even if wrong configuration was fixed. Before the error would go away only when a sync is executed.

@artem-shelkovnikov artem-shelkovnikov requested a review from a team as a code owner December 25, 2024 17:44
@artem-shelkovnikov artem-shelkovnikov changed the title Artem/reset connector status after successful ping Set connector status to CONNECTED after successful validation and ping Dec 25, 2024
Copy link
Contributor

@navarone-feekery navarone-feekery left a comment

Choose a reason for hiding this comment

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

LGTM, just have a question

Comment on lines +38 to +39
if self.reusable:
self.i = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

I can see it's only for tests so not a big deal but does this open a risk for infinite looping anywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should not be, no. Here there's no return statement, so next line that raises an error will be triggered and iterations will stop. Next time this iterator is used it's going to be iterating over same original collection, before it would immediately stop iteration.

Copy link

💚 Backport PR(s) successfully created

Status Branch Result
8.17 #3060
8.x #3061
8.16 #3062

The backport PRs will be merged automatically after passing CI.

artem-shelkovnikov added a commit that referenced this pull request Dec 27, 2024
artem-shelkovnikov added a commit that referenced this pull request Dec 27, 2024
artem-shelkovnikov added a commit that referenced this pull request Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create configuration reports validation errors
2 participants