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

✨feat(source-microsoft-sharepoint): Provide ability to sync other sites than Main sharepoint site #54658

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

aldogonzalez8
Copy link
Contributor

@aldogonzalez8 aldogonzalez8 commented Feb 24, 2025

What

Sharepoint portals are structured in a way so they can host multiple site. Currently connector only recognises files hosted in the main site and everything else is ignored.

We want to introduce the ability to sync other sites in sharepoint tenant.

How

This PR introduces a new optional site_url field to provide the URL to the sharepoint site for files to sync. If no value is provided it falls back to main site.

image

Review guide

  1. airbyte-integrations/connectors/source-microsoft-sharepoint/source_microsoft_sharepoint/stream_reader.py: new get_site_drive method to get drives from either specific site_url or main site if no URL provided.

User Impact

User will now be able to provide a sharepoint site alternative to main site.

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Copy link

vercel bot commented Feb 24, 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 Feb 28, 2025 9:55pm

@kkr2320
Copy link

kkr2320 commented Feb 25, 2025

@aldogonzalez8 - Thanks for working on it and it’s important feature. Is there a time line for this release ?

@aldogonzalez8
Copy link
Contributor Author

/appprove-regression-tests

@aldogonzalez8
Copy link
Contributor Author

aldogonzalez8 commented Feb 25, 2025

/approve-regression-tests

Check job output.

✅ Approving regression tests

@kkr2320
Copy link

kkr2320 commented Feb 25, 2025

@aldogonzalez8 - Is it possible to update the dependencies Pyarraw and requests to match the PyAirbyte version 0.23.0 ?

@aldogonzalez8
Copy link
Contributor Author

@aldogonzalez8 - Is it possible to update the dependencies Pyarraw and requests to match the PyAirbyte version 0.23.0 ?

@kkr2320 I need to document better this pr and connector official doc, after that will take a look in your request.

@octavia-squidington-iii octavia-squidington-iii removed the area/documentation Improvements or additions to documentation label Feb 27, 2025
@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Feb 27, 2025
@aldogonzalez8 aldogonzalez8 changed the title feat(source-microsoft-sharepoint): TESTING PRE-DEV for MULTI-SITE SUPPORT do not merge feat(source-microsoft-sharepoint): Provide ability to sync other sites than Main sharepoint site Feb 27, 2025
@aldogonzalez8 aldogonzalez8 self-assigned this Feb 27, 2025
@aldogonzalez8
Copy link
Contributor Author

aldogonzalez8 commented Feb 27, 2025

/approve-regression-tests

Check job output.

✅ Approving regression tests

@aldogonzalez8 aldogonzalez8 changed the title feat(source-microsoft-sharepoint): Provide ability to sync other sites than Main sharepoint site ✨feat(source-microsoft-sharepoint): Provide ability to sync other sites than Main sharepoint site Feb 27, 2025
@kkr2320
Copy link

kkr2320 commented Mar 1, 2025

@aldogonzalez8 Thanks again..
This is probably covered in your fix. But to make sure , does the fix work when Azure App privileges set to “Sites Selected” instead requiring “Read.All” . Large organizations like us , multi tenancy is a must requirement , thus site owners deploy their own application.

@aldogonzalez8
Copy link
Contributor Author

@aldogonzalez8 Thanks again..
This is probably covered in your fix. But to make sure , does the fix work when Azure App privileges set to “Sites Selected” instead requiring “Read.All” . Large organizations like us , multi tenancy is a must requirement , thus site owners deploy their own application.

I believe it should work, I do something like:

site = client.sites.get_by_url("https://contoso.sharepoint.com/sites/MySite")

That I understand translates to some GET like:

GET https://graph.microsoft.com/v1.0/sites/{hostname}:/{site-path}

That I would expect that works 🤞 for "sites.read.all" or "sites.selected" scopes in the app. Maybe I need to adjust the scope to request both and let the app grant the available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/microsoft-sharepoint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants