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

github swallows errors even if fetching issues fails #2395

Open
3 tasks
Tracked by #2393
seanstory opened this issue Apr 16, 2024 · 3 comments
Open
3 tasks
Tracked by #2393

github swallows errors even if fetching issues fails #2395

seanstory opened this issue Apr 16, 2024 · 3 comments

Comments

@seanstory
Copy link
Member

seanstory commented Apr 16, 2024

part of #2393

Description

The github connector is currently just logging a warning instead of propagating an error, if something goes wrong while fetching issues. See:

except Exception as exception:
self._logger.warning(
f"Something went wrong while fetching the repository. Exception: {exception}",
exc_info=True,
)

This error should probably be re-raised if it's not something that can be re-tried. If the github connector cannot sync any issues at all, the sync should fail. If it's just a single issue or two, that's probably ok for a WARN message.

Acceptance Criteria

  • github connector fails syncs if no issues can be synced
  • github connector attempts retries for errors that look transient
  • errors with limited scope (single documents, or edge cases) can be moved past with sufficient WARN logging
@artem-shelkovnikov
Copy link
Member

I did some digging into the code and wanted to raise the following additional issues that need to be addressed:

  1. There should be no except Exception in code. Exceptions should be properly handled - for example the git library we use has https://gidgethub.readthedocs.io/en/latest/__init__.html#gidgethub.GitHubException as base exception class - we need to switch to catching this class.
  2. This code should not raise exception, instead caller of this code must re-raise the RateLimitingError
  3. This code should re-raise
  4. This code should re-raise
  5. This code should re-raise
  6. This code should re-raise
  7. This code should re-raise

@artem-shelkovnikov artem-shelkovnikov added bug Something isn't working priority:high and removed enhancement New feature or request team:external labels Jul 4, 2024
@artem-shelkovnikov
Copy link
Member

@praveen-elastic this ticket was mistakenly marked as "enhancement" - it's a critical bug, can we prioritise fixing it?

@khushbu-elastic
Copy link
Collaborator

@DianaJourdan As we discussed in the meeting, please take a note.

NOTE: We are blocked for this issue as we are waiting for approach finalization in the similar issue - #2394
Once the finalized approach is completely implemented in that issue will prioritize this issue and work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants