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.

Sign up for GitHub

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

Jump to bottom

[outlook] Filter users to only include active ones #2967

Merged
merged 5 commits into from
Nov 18, 2024
Merged

[outlook] Filter users to only include active ones #2967

merged 5 commits into from
Nov 18, 2024

Conversation

seanstory
Copy link
Member

@seanstory seanstory commented Nov 15, 2024 •
edited
Loading

part of https://github.com/elastic/sdh-search/issues/1507

closes #2931

A failure in the outlook connector was reported with a stack trace like:

Traceback (most recent call last):
  File "/workspaces/elastic_connectors/connectors/es/sink.py", line 487, in run
    await self.get_docs(generator)
  File "/workspaces/elastic_connectors/connectors/es/sink.py", line 539, in get_docs
    async for count, doc in aenumerate(generator):
  File "/workspaces/elastic_connectors/connectors/utils.py", line 856, in aenumerate
    async for elem in asequence:
  File "/workspaces/elastic_connectors/connectors/logger.py", line 247, in __anext__
    return await self.gen.__anext__()
  File "/workspaces/elastic_connectors/connectors/es/sink.py", line 521, in _decorate_with_metrics_span
    async for doc in generator:
  File "/workspaces/elastic_connectors/connectors/sync_job_runner.py", line 454, in prepare_docs
    async for doc, lazy_download, operation in self.generator():
  File "/workspaces/elastic_connectors/connectors/sync_job_runner.py", line 490, in generator
    async for doc, lazy_download in self.data_provider.get_docs(
  File "/workspaces/elastic_connectors/connectors/sources/outlook.py", line 1058, in get_docs
    async for account in self.client._get_user_instance.get_user_accounts():
  File "/workspaces/elastic_connectors/connectors/sources/outlook.py", line 451, in get_user_accounts
    user_account = Account(
  File "/workspaces/elastic_connectors/lib/python3.10/site-packages/exchangelib/account.py", line 205, in __init__
    self.version = self.protocol.version.copy()
  File "/workspaces/elastic_connectors/lib/python3.10/site-packages/exchangelib/protocol.py", line 480, in version
    self.config.version = Version.guess(self, api_version_hint=self.api_version_hint)
  File "/workspaces/elastic_connectors/lib/python3.10/site-packages/exchangelib/version.py", line 206, in guess
    raise TransportError(f"No valid version headers found in response ({e!r})")
exchangelib.errors.TransportError: No valid version headers found in response (ErrorNonExistentMailbox('The SMTP address has no mailbox associated with it.'))

After some investigation, it appears that the user record that is causing this issue is deactivated. While this feels like a bug in the underlying exchange lib, that you can't create an Account object for a deactivated user, we can avoid this situation by filtering our list of users to active users only.

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
  • 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

Fixes a bug in the Outlook Connector where having deactivated users could cause the sync to fail.

@seanstory seanstory requested a review from a team as a code owner November 15, 2024 17:18
@seanstory seanstory mentioned this pull request Nov 15, 2024
Open
artem-shelkovnikov
artem-shelkovnikov approved these changes Nov 18, 2024
View reviewed changes
Copy link
Member

@artem-shelkovnikov artem-shelkovnikov left a comment

Choose a reason for hiding this comment

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

Good one!

@seanstory seanstory enabled auto-merge (squash) November 18, 2024 15:50
@seanstory seanstory merged commit cfe020d into main Nov 18, 2024
2 checks passed
@seanstory seanstory deleted the seanstory/filter-outlook-users-for-only-active branch November 18, 2024 15:57
github-actions bot pushed a commit that referenced this pull request Nov 18, 2024
@seanstory @artem-shelkovnikov
[outlook] Filter users to only include active ones (#2967)
29f3597 
Co-authored-by: Artem Shelkovnikov <[email protected]>
@github-actions github-actions bot mentioned this pull request Nov 18, 2024
Merged
github-actions bot pushed a commit that referenced this pull request Nov 18, 2024
@seanstory @artem-shelkovnikov
[outlook] Filter users to only include active ones (#2967)
5aec6df 
Co-authored-by: Artem Shelkovnikov <[email protected]>
@github-actions github-actions bot mentioned this pull request Nov 18, 2024
Merged
github-actions bot pushed a commit that referenced this pull request Nov 18, 2024
@seanstory @artem-shelkovnikov
[outlook] Filter users to only include active ones (#2967)
ec9b9fe 
Co-authored-by: Artem Shelkovnikov <[email protected]>
@github-actions github-actions bot mentioned this pull request Nov 18, 2024
Merged
@github-actions GitHub Actions
Copy link

💚 Backport PR(s) successfully created

Status Branch Result
✅ 8.15 #2973
✅ 8.x #2974
✅ 8.16 #2975

The backport PRs will be merged automatically after passing CI.

seanstory added a commit that referenced this pull request Nov 21, 2024
@github-actions @seanstory @artem-shelkovnikov
[8.16] [outlook] Filter users to only include active ones (#2967) (#2975
0b8006b 
)

Co-authored-by: Sean Story <[email protected]>
Co-authored-by: Artem Shelkovnikov <[email protected]>
seanstory added a commit that referenced this pull request Nov 21, 2024
@github-actions @seanstory @artem-shelkovnikov
[8.15] [outlook] Filter users to only include active ones (#2967) (#2973
67a575c 
)

Co-authored-by: Sean Story <[email protected]>
Co-authored-by: Artem Shelkovnikov <[email protected]>
seanstory added a commit that referenced this pull request Nov 21, 2024
@github-actions @seanstory @artem-shelkovnikov
[8.x] [outlook] Filter users to only include active ones (#2967) (#2974)
9a622b1 
Co-authored-by: Sean Story <[email protected]>
Co-authored-by: Artem Shelkovnikov <[email protected]>
@seanstory seanstory removed the v8.15.4 label Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artem-shelkovnikov artem-shelkovnikov artem-shelkovnikov approved these changes

Assignees
No one assigned
Labels
auto-backport release_note v8.15.5 v8.16.1 v8.17.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Outlook Connector Crashes when an SMTP address does not have a mailbox associated
2 participants
@seanstory @artem-shelkovnikov