octokit / octokit.net

A GitHub API client library for .NET
https://octokitnet.readthedocs.io/en/latest/
MIT License
2.61k stars 1.07k forks source link

Can I serialize out the results of GetAllForRepository? #1762

Closed mylodeon closed 1 week ago

mylodeon commented 6 years ago

GetAllForRepository takes a few minutes to run on our repository.

Is there a way I can serialize the results out to something that I can deserialize back in to something I can query using Linq? (Without building a wrapper around all of Octokit).

I tried a BinaryFormatter, but the Octokit.Issue isn't marked as Serializable.

shiftkey commented 6 years ago

I thought I'd be able to leverage our JSON deserializer to convert the list of issues to a string, like this:

var serializer = new Octokit.Internal.SimpleJsonSerializer();
var text = serializer.Serialize(issues);
var recreatedIssues = serializer.Deserialize<IReadOnlyList<Issue>>(text);

However it blows up on the second line with this error:

Value '' is not a valid 'ItemState' enum value.

I also tested this with JSON.NET to confirm it wasn't our deserializer to blame, but it has the same error.

Our SimpleJsonSerializer has a bunch of domain-specific rules and conventions for mapping JSON to .NET objects, but I bet we're not applying the rules enough to be able to round-trip an object through it.

ryangribble commented 6 years ago

Yeah we only serialise request models, not response models

@shiftkey the particular error you ran into is with a stringenum wrapper with an empty /unspecified value, which actually happens to be something I ran into with #1760 and am working on fixing

There could well be other round trip serialise/deserialise problems after that one is fixed though

ryangribble commented 6 years ago

@mylodeon I'm interested in doing something about your stated problem (some way to save/load our response objects) but in the meantime there potentially could be other options for you to explore to

For example which GetAllForRepository call is this (issues, PR's, commits, etc)? If you don't actually need ALL of them you can use pagination to query data in more manageable chunks, or use search Api's to specify criteria to hone in on what you are looking for (eg PR's of a certain state or issues with a particular tag or whatever)

You could also declare your own DTO class with the fields you care about and use automapper to map our responses to your DTO's which you would then be able to persist/serialise as you need to. In my case I'm doing this and storing certain objects via entity framework in a database

ryangribble commented 6 years ago

Final thought... Because we try to have immutable response classes all the parameters have protected setters and the only way to create them is via a constructor... so I wouldn't think normal desetializers like newtonsoft would be able to instantiate our objects (unless it handles classes where only the constructor can be used)? So I'm thinking using our own serializer/deserializer is probably the only way that is likely to be able to handle things easily

mylodeon commented 6 years ago

@ryangribble Thanks! I'm using this for issues. For now I am doing SearchIssues to work around it. But it does mean I can't use LINQ to form the majority of my filtering and processing - I have to format my reports effectively into a Github-compatible requests. It's fine for "pre-designed" reports, but not great for REPL-style quick queries.

DTOs with Automapper is like... hard work :). j/k. I can do that as an option if need be.

chamons commented 3 years ago

I tried this today and I ran into the same exact issue (for the same reason, GetAllForRepository is slow).

jongio commented 3 years ago

I ran into this today as well - will try SearchIssues as a workaround.

donnie-msft commented 2 years ago

For now I am doing SearchIssues to work around it.

I ran into this today as well - will try SearchIssues as a workaround.

Me too, and the workaround worked.

github-actions[bot] commented 1 year ago

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

idflyfish commented 1 year ago

I just ran into that issue when trying to serialize Team for storing in session storage. It looks like maybe I need to project the Octokit objects into my own DTOs?

nickfloyd commented 1 year ago

@idflyfish Sorry that you're running into some trouble here. Would you happen to have a snip or repro case that I could use to help chase down this issue? You should be able to use the built-in models - but since things have changed so much since we last supported this - I wouldn't be surprised if the structure of what a team looks like in the SDK would differ from what the GitHub REST API is sending over.

Let me know if you have any additional info when you get the chance. Thanks!

yfakariya commented 1 year ago

I also ran into this issue to cache the API reponse to session storage with SimpleJsonSerializer.

Following issue contained pull_request property and its state does not exists, so the serializer failed:

https://api.github.com/repos/yfakariya/msgpack-rpc-cli/issues/5

This is stacktrace:

   at Octokit.StringEnum`1.ParseValue()
   at Octokit.StringEnum`1.get_Value()
   at Octokit.Reflection.ReflectionUtils.<>c__DisplayClass35_0.<GetGetMethodByExpression>b__0(Object source)
   at Octokit.Internal.SimpleJsonSerializer.GitHubSerializerStrategy.TrySerializeUnknownTypes(Object input, Object& output)
   at Octokit.PocoJsonSerializerStrategy.TrySerializeNonPrimitiveObject(Object input, Object& output)
   at Octokit.SimpleJson.SerializeValue(IJsonSerializerStrategy jsonSerializerStrategy, Object value, StringBuilder builder)
   at Octokit.SimpleJson.SerializeObject(IJsonSerializerStrategy jsonSerializerStrategy, IEnumerable keys, IEnumerable values, StringBuilder builder)
   at Octokit.SimpleJson.SerializeValue(IJsonSerializerStrategy jsonSerializerStrategy, Object value, StringBuilder builder)
   at Octokit.SimpleJson.SerializeValue(IJsonSerializerStrategy jsonSerializerStrategy, Object value, StringBuilder builder)
   at Octokit.SimpleJson.SerializeObject(Object json, IJsonSerializerStrategy jsonSerializerStrategy)
   at Octokit.Internal.SimpleJsonSerializer.Serialize(Object item)"

I hope this info helps you!

github-actions[bot] commented 2 weeks ago

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!