octokit.net Auditing API response fields

trafficstars

We've had a number of issues raised where users have found certain fields provided by the GitHub API arent implemented in Octokit.net (mostly due to historic reasons). We have also found on more than one occasion that fields are included in Json payload responses that aren't necessarily shown in the official GitHub API docs.

I thought it would be interesting to add some trace logging to highlight any fields which are received in API responses that we don't have a matching Octokit field for, and ran as many integration tests as I could.

This is the full unaltered list... Many items in the list should likely remain un-implemented, but there could be some useful/interesting or simply overlooked fields in here. I thought I would post the results and anyone interested could highlight fields they think should be implemented in Octokit.net

Octokit.ActivityPayload

action
description
master_branch
member
pages
pusher_type
ref
ref_type
release

Octokit.ApiError

error

Octokit.ApiErrorDetail

value

Octokit.ApplicationAuthorization

app
updated_at
user

Octokit.Author

gravatar_id

Octokit.Authorization

app
token
updated_at

Octokit.AuthorizedManagementKey

pretty-print

Octokit.Blob

url

Octokit.BlobReference

url

Octokit.Branch

_links
protected

Octokit.CombinedCommitStatus

commit_url
url

Octokit.Commit

distinct
html_url

Octokit.Committer

avatar_url
events_url
followers_url
following_url
gists_url
gravatar_id
html_url
id
login
organizations_url
received_events_url
repos_url
site_admin
starred_url
subscriptions_url
type
url

Octokit.Deployment

environment
ref
repository_url
task

Octokit.DeploymentStatus

deployment_url
repository_url

Octokit.EventInfo

commit_url

Octokit.Gist

commits_url
forks_url
truncated
user

Octokit.GistFile

truncated

Octokit.GistFork

comments
comments_url
commits_url
description
files
forks_url
git_pull_url
git_push_url
html_url
id
owner
public
updated_at

Octokit.GitReference

author
comments_url
commit
committer
html_url
parents

Octokit.Issue

labels_url
repository_url
score

Octokit.IssueComment

issue_url

Octokit.License

conditions
limitations
permissions

Octokit.LicenseMetadata

featured

Octokit.Migration

archive_url
owner

Octokit.Milestone

html_url
id
labels_url
updated_at

Octokit.Organization

billing_email
description
events_url
gravatar_id
hooks_url
issues_url
members_url
public_members_url
repos_url
updated_at

Octokit.PublicKey

created_at
read_only
verified

Octokit.PullRequest

_links
comments_url
commits_url
id
merge_commit_sha
mergeable_state
merged
review_comment_url
review_comments
review_comments_url

Octokit.PullRequestReviewComment

_links

Octokit.PushEventPayload

before
distinct_size
push_id

Octokit.ReadmeResponse

_links
download_url
git_url
path
sha
size
type

Octokit.Repository

archive_url
assignees_url
blobs_url
branches_url
collaborators_url
comments_url
commits_url
compare_url
contents_url
contributors_url
deployments_url
downloads_url
events_url
forks
forks_url
git_commits_url
git_refs_url
git_tags_url
has_pages
hooks_url
issue_comment_url
issue_events_url
issues_url
keys_url
labels_url
languages_url
merges_url
milestones_url
network_count
notifications_url
open_issues
organization
public
pulls_url
releases_url
score
size
stargazers_url
statuses_url
subscribers_count
subscribers_url
subscription_url
tags_url
teams_url
trees_url
watchers
watchers_count

Octokit.RepositoryContent

_links

Octokit.RepositoryContentInfo

_links

Octokit.RepositoryContributor

gravatar_id

Octokit.RepositoryHook

last_response
type

Octokit.SearchCode

score

Octokit.Team

description
members_url
privacy
repositories_url
slug

Octokit.User

display_login
events_url
followers_url
following_url
gists_url
gravatar_id
gravatar_id
organizations_url
received_events_url
repos_url
score
starred_url
subscriptions_url
updated_at

Jun 19 '16 11:06 ryangribble

+1

Jun 28 '16 03:06 AlenPelin

Is there any progress on getting some of these fields added? I'm currently having to work around the non-existent "before" field in PushEventPayload, for example.

Jul 25 '17 10:07 Norbo11

@Norbo11 no progress per se, this wasn't a list of things to implement as many of them aren't desired/required (due to being API url fields or previously deprecated fields etc).

It's great you've found one that should be added! would you consider sending a PR to include it?

Thanks

Jul 25 '17 12:07 ryangribble

Sure, I can do that at some point this week. Also, I was wondering why, when accessing the Events API, the "deleted" field is not included in PushEvent payloads. I'm not talking about Octokit in particular, I mean the actual Events API payload, which I assumed would return the same payload as the event delivered through i.e. a web hook?

Jul 25 '17 12:07 Norbo11

I'm not sure what the actual reason is for that upstream API behaviour/design but I do see multiple references in the API docs saying that the push event is specifically different payload between the webhook and the events API

https://developer.github.com/webhooks/#payloads https://developer.github.com/v3/activity/events/types/#pushevent

I guess that confirms its expected behaviour but if you wanted your know WHY you could perhaps try contacting github support and asking?

Jul 25 '17 12:07 ryangribble

Okay, thanks a lot, I'll see what I can find.

Jul 25 '17 13:07 Norbo11

We were looking at using the distinct field on commits specifically (to cut down on processing). There's a bit of weirdness there in that it's on the webhook commit response, but not on the GET /commit response, and as best I can tell OctoKit uses the same model for both. Would that be a nullable bool (seems easier to add from a 'I could make a PR for this' standpoint)? Or would it need to be split out into two models, one for commits from the API & one for commits from a webhook payload?

Oct 19 '17 14:10 BaconSoap

How different are the response payloads? Would be great if you could post an example commit json response from both cases so we can see how many fields aren't returned.

You're right in that generally we use the same model for the same thing (eg a PullRequest) eventhough some API responses (eg a GetAll type request) don't include all of the fields that are returned from a single object Get request.

It sounds reasonable to make this field nullable (and perhaps any others that are in the same boat) rather than splitting out a new object model, unless there are heaps of discrepancies

Oct 19 '17 21:10 ryangribble

I compared what I get from a saved webhook (push event vs from the api) and added the results/diff in a gist.

Key points:

webhook uses Id, commit uses Sha
webhook has lists of files affected, api does not (and requires generating a diff)
webhook his a distinct field, commit doesn't (which makes sense given distinct is only valid at that point in time)
api has info about the tree/parents of the commit, and the webhook doesn't

(It turns out that I had actually been using a reimplemented model locally which was how I've been parsing pushes in the first place).

I think the worst difference is that it uses different property names to refer to the same thing - like id vs sha.

Oct 21 '17 14:10 BaconSoap

Maybe we should consider adding an attribute to properties that contain mappings of potential names for the deserializer. For example,

[OtherNames("sha")] 
Public string ID {get} ;

Although this will only work if the type is the same

Oct 21 '17 20:10 M-Zuber

so , how do we do these missing fields? any plans?

Mar 04 '19 08:03 niltor

Hi @niltor,

There aren't any plans to add every extra field en-masse because some of them could be being deprecated and so on...

But certainly if anyone comes across a field that is provided by the API but not defined in octokit.net it can be added to the response models via a pull request. Is there a particular missing field you've encountered?

Mar 04 '19 09:03 ryangribble

@ryangribble ok,got it.

Mar 04 '19 09:03 niltor

Deployment.Environment would be useful to have. My use case is the user can rename an enviroment and I want to reflect it in the API.

Aug 07 '20 18:08 Cyberboss

Deployment.Environment would be useful to have

@Cyberboss see https://github.com/octokit/octokit.net/pull/2560

Sep 06 '22 10:09 hansmbakker

Closing this issue, given many of these resources are now present in the current SDK. Please open new issues for other needs.

Nov 03 '22 18:11 nickfloyd

octokit.net octokit.net copied to clipboard

Auditing API response fields

octokit.net
octokit.net copied to clipboard