search

pnp / pnpframework

PnP Framework is a .NET library targeting Microsoft 365 containing the PnP Provisioning engine and a ton of other useful extensions
https://pnp.github.io/pnpframework/
MIT License
200 stars 140 forks source link

Microsoft.Graph version issue #660

Open patrikhellgren opened 2 years ago

patrikhellgren commented 2 years ago

When using PnP.Framework 1.9 (which references Microsoft.Graph version 3.33) in a project that itself references Microsoft.Graph (latest stable 4.28) there are some issues when calling PnP.Framework methods that uses Microsoft.Graph (e.g. all methods in the UnifiedGroupsUtility class). I get an exception saying "Method not found".

The issue goes away if either updating PnP.Framework to use Microsoft.Graph 4.28 or downgrading our project to use 3.33.

Is there a known way of having PnP.Framework not be that version dependant for Microsoft.Graph? If not, @jansenbe would it be ok to bump the Microsoft.Graph NuGet package (and it's dependencies) to the latest stable (3.33 is after all a year old)? I would gladly submit a PR for that since I already have it finished as a test.

Environment: PnP.Framework 1.9 (also tried with latest daily) .Net 6 Tested both in an Azure Function v4 (Windows) and in LINQPad 7 running on Windows 11 with the same results.

jansenbe commented 2 years ago

Hey @patrikhellgren : Adding @KoenZomers/ @gautamdsheth / @erwinvanhunen here (Koen is one of the main authors of the PnP Framework code using Graph SDK) + for PowerShell impact. Overall this seems to be a simple bump, but we need to carefully review this as we'll want shared dependencies between PnP Core, PnP Framework and PnP PowerShell to stay aligned. Also from the PS side bumping certain dependencies might lead to assembly load conflicts when PS is used in Azure. So the exercise to do is a global "review and bump" dependencies for PnP Core / PnP Framework and PnP PowerShell

patrikhellgren commented 2 years ago

@jansenbe the mention of KoenZomers is not correct so adding him here again. @KoenZomers please check Bert's previous comment in this thread.

KoenZomers commented 2 years ago

I believe to remember we were having some weird constraints which made us be stuck on an old Graph version. Actually believe to have heard this from you in the past @jansenbe :) My personal preference would be to disconnect entirely from the Microsoft Graph SDK and just use MSAL for the authentication and make raw Graph calls using a HttpClient. I don't see what it adds to use the Graph SDK. From PnP PowerShell perspective, in version 2, we want to break free from a lot of these dependencies by no longer natively passing back framework entities but creating mappers and returning our own entities. This will allow much greater flexibility.

For now, I'd say @patrikhellgren give it a try and bump the Graph SDK version and see if things will even compile. If you don't mind running some more extensive testing on the Graph backed functionality in PnP Core to discover if all works well, I guess we can consider upgrading at least in a nightly build. This will allow us and a bit larger audience to test drive this in a bit larger scale before we publish a next release.

Would this be an okay approach to you as well @jansenbe?

jansenbe commented 2 years ago

@KoenZomers : I'm all for lowering our dependencies and if so moving away from the Graph SDK. MSAL based auth is already part of PnP Framework and there's a base REST client that can be used (think that's how we initially did this). This however will require a rewrite of all the code that depends on the SDK. Also PnP PS has a direct reference to this Graph SDK, so that would also be reviewed for usage and if needed updated.

I personally don't have the bandwidth to work on these rewrites, but if @KoenZomers / @patrikhellgren / @gautamdsheth or others have some time then I suggest doing the work to move PnP Framework and PnP PowerShell away from Graph SDK. PnP Core SDK is not using it. If the needed Graph usage in PnP Framework is Teams related, then using these features from PnP Core SDK might save some time.

Once that's done we can evaluate bumping the auth dependencies for the whole stack (PnP Core, PnP Framework and PnP PowerShell)

luismanez commented 2 years ago

Is there any update here? we´re also facing this issue. Our project is using a pretty up to date version of the package Microsoft.Identity.Web.MicrosoftGraph, which is referencing Microsoft.Graph 4.11

We´ve done huge effort to move our product to .NET 6, Azure Functions v4 and latest packages, and is a bit painful to be now in this situation.

I also don´t understand why this happens, as in theory, PnP is not stuck to a specific version in Graph SDK (like it was in the past with Newtonsoft). Image below says it should support Microsoft.Graph 3.33 or greater:

image

Any update would be highly appreciated.

martinlingstuyl commented 1 year ago

Any updates here? We're also facing this issue. Just after rewriting our code to use the latest Microsoft Graph SDK... (5.11.0)

martinlingstuyl commented 1 year ago

It seems to me Microsoft Graph SDK is the new Newtonsoft. @KoenZomers any update? @luismanez what did you do to work around this?

luismanez commented 1 year ago

There are no updates. They need more hands to remove the dependency on the SDK, which is not easy and requires a lot of tests.

Our "workaround" was to download the source code, and upgrade SDK Graph NuGet package in PnP (and we're still in SDK 4.x), which obviously is not ideal.

Also note that if you're using SDK 5.x, it has quite a few breaking changes from 4.x (you know this, as you have re-written your code), so is not enough with upgrading PnP SDK package, and you're gonna need to change code in the PnP project...

:(

patrikhellgren commented 1 year ago

I never got around to doing any extensive testing after bumping the dependency on Microsoft.Graph in PnP.Framework to anything later than 3.33 so we are still using that version in our solutions which of course is not optimal. We will probably also need the additions in the later versions shortly so great if this could get some attention again.

martinlingstuyl commented 1 year ago

Also note that if you're using SDK 5.x, it has quite a few breaking changes from 4.x (you know this, as you have re-written your code), so is not enough with upgrading PnP SDK package, and you're gonna need to change code in the PnP project...

Yes, I had just rewritten them before trying it out and discovering the pnp.framework issue 😔 fortunately, it wasn't a big job. I saved the changes somewhere so I can reuse them as soon as possible.

gautamdsheth commented 1 year ago

Hey @martinlingstuyl , @patrikhellgren - have created a draft PR to bump the version ?

https://github.com/pnp/pnpframework/pull/859

martinlingstuyl commented 1 year ago

That's helpful!

gautamdsheth commented 1 year ago

It is 95% complete, there's 1 method is needs to be refactored and after that, will test it out.

andrewbackway commented 1 year ago

Thanks for the information, any update?

TomekPi commented 9 months ago

@gautamdsheth , any update on this? Are you targeting the Graph SDK 5.x or 4.x?

nsyng commented 8 months ago

@patrikhellgren @gautamdsheth any update on this?

raclettierer commented 4 months ago

any news ?

TashunkoWitko commented 3 months ago

@gautamdsheth @jansenbe I wonder what is the way forward here. Is it ever gonna be resolved or is this a gap that we are gonna (have to) live with? The release of Graph SDK v5.0 was over a year ago...

See also #874

quails4Eva commented 3 months ago

Based on the earlier discussion about removing the Graph SDK nuget dependency I created a pr that starts switching away from the nuget package. @jansenbe does what I've done so far look ok? If so I can try to keep picking away at it as I have time. #1003