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

Incomplete schema mapping from GitHub API to Owner type #50

Closed
jiahao opened this issue Mar 16, 2016 · 7 comments
Closed

Incomplete schema mapping from GitHub API to Owner type #50

jiahao opened this issue Mar 16, 2016 · 7 comments

Comments

@jiahao
Copy link
Contributor

jiahao commented Mar 16, 2016

The GitHub.Owner type is missing several fields which can be returned from a GitHub API query returning a representation of a GitHub user.

For example, a GitHub API query to https://api.github.com/repos/JuliaLang/julia/contributors (as would be accessed by contributors("JuliaLang/julia") returns records in raw JSON of the form

  {
    "login": "jiahao",
    "id": 1732,
    "avatar_url": "https://avatars.githubusercontent.com/u/1732?v=3",
    "gravatar_id": "",
    "url": "https://api.github.com/users/jiahao",
    "html_url": "https://github.com/jiahao",
    "followers_url": "https://api.github.com/users/jiahao/followers",
    "following_url": "https://api.github.com/users/jiahao/following{/other_user}",
    "gists_url": "https://api.github.com/users/jiahao/gists{/gist_id}",
    "starred_url": "https://api.github.com/users/jiahao/starred{/owner}{/repo}",
    "subscriptions_url": "https://api.github.com/users/jiahao/subscriptions",
    "organizations_url": "https://api.github.com/users/jiahao/orgs",
    "repos_url": "https://api.github.com/users/jiahao/repos",
    "events_url": "https://api.github.com/users/jiahao/events{/privacy}",
    "received_events_url": "https://api.github.com/users/jiahao/received_events",
    "type": "User",
    "site_admin": false,
    "contributions": 716
  },

Several of these fields, notably the *_url fields like avatar_url, have no corresponding field representation in the GitHub.Owner type. Unfortunately, the lack of this information completely breaks code that relied on having this data, such as the World of Julia notebook (which wants the avatar_url fields).

cc @jrevels

@jiahao
Copy link
Contributor Author

jiahao commented Mar 16, 2016

The complete list of missing fields is

    avatar_url
    followers_url
    following_url
    gists_url
    starred_url
    subscriptions_url
    organizations_url
    repos_url
    events_url
    received_events_url
    contributions

Unfortunately, contributions doesn't make sense as an attribute intrinsic to a user account, unless it is explicitly related to the corresponding repo. So maybe this will have to be stored as Dict("JuliaLang/julia" => 716) within the Owner type in the example of the OP.

jiahao added a commit to jiahao/GitHub.jl that referenced this issue Mar 16, 2016
@jiahao
Copy link
Contributor Author

jiahao commented Mar 16, 2016

Oh never mind, I see that contributions is mapped to the value of the Dict that is output_of_ contributors[1][i].

jiahao added a commit to jiahao/GitHub.jl that referenced this issue Mar 16, 2016
@jrevels
Copy link
Member

jrevels commented Mar 16, 2016

Yeah, I left out a good chunk of the *_url fields from various types because most of that info can be naturally obtained/interacted with via the other API methods, but that was probably the wrong decision (also, it left some holes e.g. avatar_url).

Feel free to open a PR with the fix you added.

KristofferC pushed a commit to KristofferC/GitHub.jl that referenced this issue Jul 22, 2017
KristofferC pushed a commit to KristofferC/GitHub.jl that referenced this issue Jul 22, 2017
@Keno
Copy link
Contributor

Keno commented Jul 31, 2017

avatar_url is just https://avatars.githubusercontent.com/u/$(get(owner.id)). I don't think we should represent these urls in the types, because we also hardcode them in the various accessor methods. I'd rather be consistent about than end up with a weird mix.

@jrevels
Copy link
Member

jrevels commented Jul 31, 2017

I'd rather be consistent about than end up with a weird mix.

I'm still not opposed to this change (it's better to hew closer to GitHub's actual schema when possible), but this is a good point. We should add the URL fields for every GitHubTypes all at once to avoid confusion or not make the change at all.

cc @KristofferC, since you have a PR for part of this

@Keno
Copy link
Contributor

Keno commented Jul 31, 2017

What I meant is that we hardcode paths e.g. here: https://github.com/JuliaWeb/GitHub.jl/blob/master/src/owners/owners.jl#L78
with this change, should we change that to

gh_get_paged_json(owner.orgs_url)

If so, that doesn't work with skinny objects any more. If not, then why do we want to expose those fields? We could compute them on-demand from the hardcoded strings.

@KristofferC
Copy link
Collaborator

Not really since the API is field access?

@Keno Keno closed this as completed Oct 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants