Skip to content

feat(source-microsoft-lists): fix ListItems and ContentValues #58575

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

Merged
merged 8 commits into from
Apr 25, 2025

Conversation

aldogonzalez8
Copy link
Contributor

@aldogonzalez8 aldogonzalez8 commented Apr 20, 2025

What

Fix the ListItem and ContentValues streams that have errors when attempting to fetch certain records.

How

They were doing a Cartesian product "Lists" X "Items", no matter if the item didn't belong to the List, which was leading to errors like:

columnvalues: MessageRepresentationAirbyteTracedErrors('\'GET\' request to \'https://graph.microsoft.com/v1.0/sites/site-id/lists/list-id/items/item-id?expand=fields\' failed with status code \'404\' and error message: \'The specified list item was not found\'. Request (body): \'None\'. Response (body): \'{\'error\': {\'code\': \'itemNotFound\', \'message\': \'The specified list item was not found\', ...

We are also adding FAST aribyte standard tests.

Review guide

User Impact

Users can nos sync these streams without errors

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

@aldogonzalez8 aldogonzalez8 self-assigned this Apr 20, 2025
Copy link

vercel bot commented Apr 20, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 25, 2025 9:18pm

Copy link
Collaborator

@aaronsteers aaronsteers left a comment

Choose a reason for hiding this comment

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

This looks great, @aldogonzalez8 ! One change requested. After this, I hope to see the FAST tests running under the "Unit tests" category of the airbyte-ci report.

image

Comment on lines 30 to 36
- suite: unitTests
testSecrets:
- name: SECRET_SOURCE-MICROSOFT-LISTS_CREDS
fileName: config.json
secretStore:
type: GSM
alias: airbyte-connector-testing-secret-store
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shared in DM but posting here for posterity...

Apparently unitTests don't support secrets. And manifest-only connectors don't support integration tests...

Suggestion is to move these into integration tests and add that capability (integration tests for manifest-only connectors) into airbyte-ci in a future iteration.

Copy link
Collaborator

@aaronsteers aaronsteers left a comment

Choose a reason for hiding this comment

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

Looks great, @aldogonzalez8! Thanks for your help in testing the new tests!

@aldogonzalez8 aldogonzalez8 merged commit af97d3e into master Apr 25, 2025
27 checks passed
@aldogonzalez8 aldogonzalez8 deleted the ac8/source-microsoft-lists/fix-streams branch April 25, 2025 21:31
ericvanbenschoten-dv01 pushed a commit to ericvanbenschoten-dv01/airbyte_ev that referenced this pull request May 6, 2025
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.

3 participants