search

aj-r / RiotNet

A .NET/C# client for the Riot Games API
MIT License
23 stars 10 forks source link

V3 deprecation #30

Closed dfernandezramos closed 5 years ago

dfernandezramos commented 5 years ago

Hi there,

My name is David and I use RiotNet for my app. I would like to know if someone is working on the new V4 endpoints since V3 ones will be deprecated on 28th January.

Thank you so much in advance!

aj-r commented 5 years ago

Thanks for letting me know, I haven't been watching the changes. I dont have a lot of time right now but I will happily accept PRs.

On Mon, Dec 3, 2018, 7:10 AM David Fernández <notifications@github.com wrote:

Hi there,

My name is David and I use RiotNet for my app. I would like to know if someone is working on the new V4 endpoints since V3 ones will be deprecated on 28th January.

Thank you so much in advance!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/aj-r/RiotNet/issues/30, or mute the thread https://github.com/notifications/unsubscribe-auth/AGpes--xsXdMe1127A2H8N9jqarGmhuaks5u1RSqgaJpZM4Y-dSP .

dfernandezramos commented 5 years ago

Good! I'll try 👍

Also, can you tell me if there is a special setup to work with in local? When I'm trying to open the project on Windows (Visual Studio 2015) I get the next message loading the project:

"The default XML namespace of the project must be the MSBuild XML namespace. If the project is authored in the MSBuild 2003 format, please add xmlns="http://schemas.microsoft.com/developer/msbuild/2003" to the element. If the project has been authored in the old 1.0 or 1.2 format, please convert it to MSBuild 2003 format. "

Any idea with that? Thanks a lot!

jessehallam commented 5 years ago

@dfernandezramos I, too, have an interest in getting V4 APIs implemented. Might be worth having a short conversation on Skype or Discord to divide up the labour accordingly.

FYI I was able to download the project and build in Visual Studio 2017 immediately with no effort at all. I'd recommend upgrading to VS2017.

aj-r commented 5 years ago

Yes you'll definitely need VS2017 (community edition should be fine).

If you find a good way to split up the work, I'd suggest creating GitHub issues and assigning them to yourselves. That way others (including myself) can contribute if they get time. If you don't have permission for some reason then let me know and I'll fix it.

On Mon, Dec 3, 2018, 3:36 PM jessehallam <notifications@github.com wrote:

@dfernandezramos https://github.com/dfernandezramos I, too, have an interest in getting V4 APIs implemented. Might be worth having a short conversation on Skype or Discord to divide up the labour accordingly.

FYI I was able to download the project and build in Visual Studio 2017 immediately with no effort at all. I'd recommend upgrading to VS2017.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/aj-r/RiotNet/issues/30#issuecomment-443860997, or mute the thread https://github.com/notifications/unsubscribe-auth/AGpes1GkV63d3C0oLxyYInuXrcpsyjdFks5u1YtdgaJpZM4Y-dSP .

aj-r commented 5 years ago

What changes do we need to make from V3 to V4 anyway? The APIs look very similar. At first glance it looks like we need to:

It might be good to include the 2019 changes in the next release, too.

dfernandezramos commented 5 years ago

@jessehallam I will be working on this in the next weeks. If I need help or I can not continue working on it I'll let you know. Thanks a lot.

@aj-r Just one more question. I have added two txt files with my api-key (the same key for both files, I don't know if there is a problem with that) and 52 tests are failing mostly for a forbidden response. I have tested the same request in the browser and the response is the same. For example: https://euw1.api.riotgames.com/lol/platform/v3/champions/103?api_key=HERE_YOUR_API_KEY

I've tested with different platforms and different API keys. Am I missing something? Thanks!

aj-r commented 5 years ago

Has your API key expired? Your dev key expires in 24h.

On Tue, Dec 4, 2018, 5:27 AM David Fernández <notifications@github.com wrote:

@jessehallam https://github.com/jessehallam I will be working on this in the next weeks. If I need help or I can not continue working on it I'll let you know. Thanks a lot.

@aj-r https://github.com/aj-r Just one more question. I have added two txt files with my api-key (the same key for both files, I don't know if there is a problem with that) and 52 tests are failing mostly for a forbidden response. I have tested the same request in the browser and the response is the same. For example:

https://euw1.api.riotgames.com/lol/platform/v3/champions/103?api_key=HERE_YOUR_API_KEY

I've tested with different platforms and different API keys. Am I missing something? Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aj-r/RiotNet/issues/30#issuecomment-444050740, or mute the thread https://github.com/notifications/unsubscribe-auth/AGpes-MWHKP6UlFfMZ7M_lLMU8uxCD2pks5u1k4YgaJpZM4Y-dSP .

dfernandezramos commented 5 years ago

The dev key is not expired. I've checked it before start testing... 🤔 and as I told you, I have even tested it with other API keys

dfernandezramos commented 5 years ago

Ok now I see that Riot is doing weird things like return a Not Found response on V4 but not on V3. I think this is maybe related to since some models has changed they need to migrate their database for the old stored data. Otherwise I don't know what's happening.

Anyway I'm fixing it by making new requests for data in this season

aj-r commented 5 years ago

Ok yeah I'm getting a 403 for some tests, too (but not all - some tests like GetSummonerBySummonerIdTest still work). In fact it seems like most of the issues are in the static data API, so yeah it could be that we're just using out-of-date data.

I created a new branch: apiv4. Please merge your V4 PRs into that branch, and then I'll merge it into master once it's ready.

It would be nice to have several small-ish PRs instead of one massive PR for V4. I feel like the most logical thing is to have a separate PR for each API? Do whatever makes the most sense though.

dfernandezramos commented 5 years ago

Hi there @aj-r I have made my PR 😄 Here you are: https://github.com/aj-r/RiotNet/pull/31

aj-r commented 5 years ago

Thanks! I'll try to look at it this evening.

On Wed, Dec 5, 2018, 7:54 AM David Fernández <notifications@github.com wrote:

Hi there @aj-r https://github.com/aj-r I have made my PR 😄 Here you are: #31 https://github.com/aj-r/RiotNet/pull/31

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aj-r/RiotNet/issues/30#issuecomment-444475191, or mute the thread https://github.com/notifications/unsubscribe-auth/AGpes0McQj7tpogBC4VHkAvudH6iGjxbks5u18IJgaJpZM4Y-dSP .

aj-r commented 5 years ago

@dfernandezramos I don't think this is completely done - we still need to add a new endpoint to get a summoner by PUUID (or did I miss that in your PR?)

dfernandezramos commented 5 years ago

True! I'll try to develop it when I can. Sorry!

dfernandezramos commented 5 years ago

Here you are! https://github.com/aj-r/RiotNet/pull/33

aj-r commented 5 years ago

@dfernandezramos Is this done now? Should I release to nuget?

dfernandezramos commented 5 years ago

Hey @aj-r . I've been using a nuget released with my fork and it seems it works. I've not finished with my project migration yet. This week I will do an internal release to check everything is OK and I won't release to production until 27-28th Jan because that is the Riot's official V4 release date.

Up to you! 😄

aj-r commented 5 years ago

Ok. If everything seems stable then I'll release soon. I want to release before the depreciation date so devs have time to update their code.

On Mon, Jan 7, 2019, 2:43 AM David Fernández <notifications@github.com wrote:

Hey @aj-r https://github.com/aj-r . I've been using a nuget released with my fork and it seems it works. I've not finished with my project migration yet. This week I will do an internal release to check everything is OK and I won't release to production until 27-28th Jan because that is the Riot's official V4 release date.

Up to you! 😄

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aj-r/RiotNet/issues/30#issuecomment-451847730, or mute the thread https://github.com/notifications/unsubscribe-auth/AGpesz8IfFOLk-7sKoT0jJRBy4tP6T97ks5vAvqrgaJpZM4Y-dSP .

jessehallam commented 5 years ago

I've been using the V4 APIs using this library since December and all has been well on my end, in production use.

I will point out that I had to manually bring the V3 APIs back in so that I could migrate my existing users to the new V4 ID/PUUID formats: Use V3 to get the most current summoner name, then use V4 to request the IDs by summoner name.

aj-r commented 5 years ago

Released to NuGet as v7.0.0