-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this 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.

- suite: unitTests | ||
testSecrets: | ||
- name: SECRET_SOURCE-MICROSOFT-LISTS_CREDS | ||
fileName: config.json | ||
secretStore: | ||
type: GSM | ||
alias: airbyte-connector-testing-secret-store |
There was a problem hiding this comment.
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.
There was a problem hiding this 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!
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 theList
, which was leading to errors like: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?