Coder Social home page Coder Social logo

discussions's People

Contributors

bkeepers avatar gr2m avatar nickfloyd avatar octokitbot avatar ryangribble avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar

discussions's Issues

Where to direct support requests

I've been reading through the open issues in the octokit library, and it looks like we're getting a fair number of support requests. Is this the right place for them? If not, what is a better place?

I'd suggest the platform.github.community forum, but that seems to be directed at the APIs and various types of GitHub integrations.

Maintainers sync - Nov 29 2017

Attendees:

Agenda:

  • Introductions
  • Overall motivation & goals
  • Octokit Program (TL;DR: complete and officially supported SDKs in the most popular languages)
    • How can we make the Octokit Maintainer’s life easier?
    • How can we make it easier to contribute to Octokits, especially for integrators?
    • Beyond REST: GitHub Apps, OAuth, webhooks, GraphQL
    • Octokit community maintainers
  • Sharing resources
    • REST Routes? (see my comment at +Octokit Program)
      • Philosophy around preview periods: things will change based on feedback, so they should not be considered production-ready
      • Need to close out preview features more quickly
      • conclusion: worth the near term investment, even if REST will be depreciated in ~1 yr
    • Fixtures (For Octokits, integrators (octokit/fixtures#71)
    • Errors
    • Documentation (sth like this? https://gr2m.github.io/slate, language snippet for each endpoint)
    • Cookbook (Create content we can point to for recurring “How do I…?” requests)
    • Glossary
    • LTS (Enterprise) versions?
  • Q&A
    • REST API
      • When will the REST API be deprecated?
      • What will happen to the previews?
    • GraphQL
      • How should authentication APIs look like? Differences to REST (e.g. passing client_id/_secret as query arguments)?
    • As a community maintainer, how to ask questions about GitHub platform / internals?
    • …, how to report bugs?

Open issues to discuss:

  • Plan for octokit.github.io
  • Licenses for GHE
  • How do you know what APIs a GHE instance supports? Log? LTS Octokit Versions? (@gregor)
  • LTS client versions tied to GHE instances?
  • GraphQL
  • Everyone check out and give feedback on octokit/maintainers#3

How to handle "ENOTFOUND api.github.com" responses

I’m looking into implementing best practises right into octokit.js as much as possible, like limiting mutating requests to 1 per second by default.

When I looked into the logs of the WIP app, I’ve found that while there are plenty "You have triggered an abuse detection mechanism" errors, there are many more errors like this

request to https://api.github.com/repos/gr2m/sandbox/statuses/b2d29b79ad66e9fc589e0c9f73319b2f5293d4fa failed, reason: getaddrinfo ENOTFOUND api.github.com api.github.com:443

image

Is that something any of you encountered before? Might it be a problem with now? Would you implement a retry for that error directly into the library?

Include a Python library

Python is one of the top languages on GitHub and we should probably try to find a library that works well and provide resources to help it out.

A quick look has given a few Python libraries:

I'm not sure what the requirement is for a library being included in this repo though. I bet like most open source projects, they could do with some help. Has anyone previously reached out to an Python library authors?

Webhook support in octokit.js

We recently adopted node-github and will be renaming it to octokit.js in the coming months (#1). As part of that effort, we'd like to include webhook support in octokit.js.

https://github.com/rvagg/github-webhook-handler is the defacto module for node. @rjz has been maintaining it lately, but can't actually release new versions because he doesn't have access to the npm module from @rvagg. I'd like to propose that we fold it into octokit.js. There are still some things to figure out with how octokit.js will be structured with regard to plugins like this, but I imagine it will work something like this from an enduser's perspective:

  1. $ npm install octokit will install all of octokit with all dependencies, and require('octokit).webhooks will give you access to the webhooks module
  2. $ npm install @octokit/webhooks will install just the webhooks module, and require('@octokit/webhooks') will give you access to it.

Thoughts?

I've forked it to octokit.js-webhooks and started submitting some PRs. I will work with @gr2m to figure out the plan for getting it released along with octokit.js.

cc @rjz @zeke @gr2m

Caching: handling 304 responses

This is a follow up for octokit/octokit.js#673 (comment) /cc @jsg2021

The documentation on Conditional requests says

Most responses return an ETag header. Many responses also return a Last-Modified header. You can use the values of these headers to make subsequent requests to those resources using the If-None-Match and If-Modified-Since headers, respectively. If the resource has not changed, the server will return a 304 Not Modified.

Maybe more importantly for us, it says

Note: Making a conditional request and receiving a 304 response does not count against your Rate Limit, so we encourage you to use it whenever possible.

(this should probably be added to the documentation on Best practices for integrators?)

My question is: do you / should we handle caching as part of the Octokit libraries?

I would say yes, the same way there should be APIs for pagination or handling of rate limits. I’m curious what we currently do in the other implementations

Best practices for Throttling / Rate Limiting

When consuming content from the GitHub API, it's pretty easy to hit your rate limit or be throttled for sending too many concurrent requests. Every language has tools for throttling function calls (e.g. limiter for node), but as a GitHub API consumer it's kind of cumbersome to find the right tool for doing this, then figure out how to make it work nicely with your GitHub client.

I think it would be neat if some clients had a built-in mechanism for throttling requests so users wouldn't have to think about it (as much) when writing their code.

Does anyone know of existing clients or patterns for doing this well?

v3 REST API: Inconsistencies in DELETE endpoints with request bodies

In total there are 7 DELETE routes that accept a request body. 4 of these expect the body to be the JSON string of the array input. The remaining 3 use namespacing for the input variables.

One of these 3 has only a single variable: Remove assignees from an issue. The other two have multiple input parameters, so it wouldn’t work without namespacing:

The 4 routes that expect the JSON string to be the input value only have a single input parameter:

I would suggest use namespace input parameters for all endpoints.

This could be done without breaking the API, the route handler could detect if it receives an object or an array (or single string in some cases) and handle it accordingly.

This inconsistency makes things a bit harder for the developers of Octokit libraries for the REST API, as they have to hardcode these exceptions. An alternative would be to call the parameter input for some while e.g. labels for others, but that would be confusing and again, inconsistent.

Transition plan for node-github

@bkeepers, @kytrinyx, @gr2m, and I are working with the maintainers of the node-github module to make it an official octokit. Let's outline a plan for finding a new home for the project to thrive.

  • Get approval from maintainers @mikedeboer and @kaizensoze
  • Give GitHubbers commit access to GitHub repo and npm module
  • Choose a name for the new repo (node-github, octokit.js, octokit-js, ...): octokit.js
  • Choose a name for the new npm module (github, octokit, ...): octokit
  • Transfer repo from @mikedeboer's personal account to the @octokit org.
  • Redirect docs from mikedeboer.github.com/node-github to octokit.github.io/octokit.js or similar.

Best practices for making it easy for contributors to run tests

The Ruby client uses VCR to create cassettes, but getting everything running in order to make a new cassette is non-trivial. (See example here: octokit/octokit.rb#895)

This is non-trivial even for maintainers. E.g. we don't have access to an enterprise instance to test against.

It would be nice if there were a shared format for recorded API calls that all the libraries could use.

In the absence of something like that, perhaps we could have a bot who creates recorded calls (authenticated by the user asking for it in order to avoid spammy users or rate limit abuse).

Get archive link API: return URL or full archive content?

This is a follow up question to octokit/octokit.js#560 (comment) /cc @paulmelnikow

The documentation of GET /repos/:owner/:repo/:archive_format/:ref says

This method will return a 302 to a URL to download a tarball or zipball archive for a repository

In the best "Best practices for integrators" the documentation recommends to Follow any redirects that the API sends you. It references the HTTP Redirects section where it says for 302 redirects:

Temporary redirection. The request should be repeated verbatim to the URI specified in the Location header field but clients should continue to use the original URI for future requests.

On the other side, API is called "Get archive link", so in this case I wonder if we should diverge from the documentation in Octokit libraries and instead return the archive link, instead of full archive content.

The current implementation in node-github returns the full archive as string

const result = github.repos.getArchiveLink({
  owner: 'octokit',
  repo: 'node-github'
})
// result is full content of `octokit/node-github` master branch tarball

What @paulmelnikow suggests instead

const result = github.repos.getArchiveLink({
  owner: 'octokit',
  repo: 'node-github'
})
// result is full URL to download `octokit/node-github` master branch tarball

Let me know what you think :)

GraphQL & query injection attacks

I’ve created a minimal-as-possible GraphQL library for the JavaScript Octokit: https://github.com/octokit/graphql.js

Teddy brought up the issue that the simple API design does not prevent users of the library to create code that is vulnerable to query injection attacks:
octokit/graphql.js#2

GraphQL has the concept of variables built into its design which prevents this, but users need to be disciplined enough to use the variables instead of just creating a string with the values replaced.

JavaScript supports an API that would prevent that: tagged templates. I’m not sure if something similar is possible for other languages.

The use of tagged templates would work but also complicates the API surface a little, I’m not yet sure if it’s necessary, or if there are other ways, such as a good documentation, linting, etc.

I wonder if anybody has thoughts on this?

Suggestion: show some metadata in list of third-party libraries

The list is useful, but it could be made even more useful by integrating it with the GitHub API (you see what I did there?):

The page could display, alongside each library, some stats from their repos:

  • The number of stars
  • The number of contributors
  • The number of releases
  • The date of the most recent release
  • The date of the most recent commit to master
    • Since not all projects use GitHub’s releases feature

and I’m sure there are some other stats that might be helpful.

(BTW it would be really cool if y’all had some kind of embeddable badge that’d display this info.)

GitHub-specific URI template parsing

I’ve created a custom URL template parsing library: https://github.com/gr2m/octokit-rest-url-template/blob/initial-version/README.md (not yet released). The main reasons why are

  1. :varname placeholders are not part of RFC 6570, but I’d like it to be able to resolve both :varname and {varname} placeholders in the URL
  2. I needed the list of used variable names, as unused options are passed as query parameters or as request body, see below
  3. minimal bundle size for usage in browser. It only implements the parts of RFC 6570 that GitHub is actually using, based on all responses shown on the GitHub API docs, see full list of GitHub’s URL template variables

I’d like to move that repository to the @octokit organization and release it as @octokit/rest-url-template, unless there are any objections?

The longer story

I’ve been thinking a lot about what an Octokit Client library for GitHub’s REST API is at its core, to inform the refactoring of our current Node library: octokit/octokit.js#692

My conclusion is that at it’s very core it’s turning the options shown in the API docs for an API endpoint into generic request options. Taking List organization repositories as an example, this core method turns these options

method GET
url /orgs/:org/repos
org (URL variable) e.g. octokit
type (endpoint parameter) e.g. all
per_page (pagination parameter) e.g. 20
page (pagination parameter) e.g. 2

into generic request options that can be passed into any request library or native request API

method GET
url e.g. https://api.github.com/orgs/octokit/repos?type=private&per_page=20&page=2

The way it works is the following

  1. method, url and headers are passed through to request options
  2. Placeholders in url are replaced using the remaining options
  3. After that if method is GET or HEAD, all remaining options are passed as URL query param option, otherwise remaining options are sent as JSON string in the request body, see documentation on parameters. There are a few exceptions, but that is the general rule.

The URL template parsing is at the very heart of it and I think that at least for Node, it would be nice to have an officially supported URL template parsing library, because there are so many out there

Best practices for calling related URLs

I don't know if there are any best practices for this.

One of the things that the GitHub REST API does well, is that it tells you what URLs you can call for a given object.

E.g. this pull request event:

{
  "action": "closed",
  "number": 3,
  "pull_request": {
    "url": "https://api.github.com/repos/exercism/test-maintainer-sync/pulls/3",
    "id": 128870299,
    "html_url": "https://github.com/exercism/test-maintainer-sync/pull/3",
    "diff_url": "https://github.com/exercism/test-maintainer-sync/pull/3.diff",
    "patch_url": "https://github.com/exercism/test-maintainer-sync/pull/3.patch",
    "issue_url": "https://api.github.com/repos/exercism/test-maintainer-sync/issues/3",
    "number": 3,
    "state": "closed",
    ...
}

I guess it doesn't help that it's not obvious which URLs are going to return HTML and which will return JSON. The diff_url is plain text. Well, first it's a redirect that you'd have to follow, then it's plain text.

Anyway. I wonder if it would be useful to let people use the client to follow URLs.

A JavaScript Octokit

JavaScript is the world's most popular programming language, but it doesn't yet have an octokit. We want to change that!

There are lots of JavaScript clients for the GitHub API out there in the wild already. Like, hundreds. Instead of creating a new client from scratch, let's look at what's out there and see if there's already a client that could fit the bill.

Considerations

The following things should be taken into account when evaluating a client:

  • Usability - is the client intuitive to use? Can users guess the DSL once they get the hang of it?
  • Completeness - does the client support everything that GitHub's REST API offers?
  • Flexibility - can the client be adapted to support new GitHub API features like GraphQL?
  • Developer Experience - how easy is it for folks to contribute to the project?
  • License - permissive enough?

Other considerations that are kind of unique to Javascript:

  • Control flow - does the client support Promises? old-school node-style callback? Both?
  • Browser support - should the client work in browsers too? How does auth work?
  • Node version support - how far back should we support? See nodejs/LTS

Candidates

We've used a number of clients in our projects at GitHub, including gh-got, ghutils, octokat, and others. These clients range from minimal to fully-loaded, and each has their own strength.

The client that stands out (to us) as the most "feature-complete" is github. It is not only the most popular (by download count and direct dependent count), but it is also generated by a schema, which seems like a promising design approach for being able to sustainably grow and maintain the client over time.

Here's how it stacks up given the above considerations:

  • ✅ Usability - intuitive and doesn't try to be too magical, e.g. github.repos.getCollaborators
  • ✅ Completeness - client is regularly updated as GitHub enables new API features.
  • ✅ Flexibility - schema-based design.
  • ❓Developer Experience - dev setup is not currently documented
  • ✅ Control flow - bring our own Promise implementation or use node-style callbacks.
  • ✅ License - MIT
  • ❓Browser support - not sure
  • ✅ Node version support - support back to 0.4.0!

Feedback, please.

This repo was created to open a dialogue with the open source community about how octokits should be chosen, designed, and maintained. Please feel free to comment here about things I've overlooked, other factors we should be considering, or another JS client that you think would make a great octokit.

Inconsistency in issue label endpoints in rest.js

@gr2m I ran into an interesting inconsistency today, and I don't think there's an obvious correct answer.

The TL;DR is that the "add labels to issue" and "remove label from issue" endpoints take similar parameters (owner, repo, number) with a fourth key that differs:

  • addLabels takes labels
  • removeLabel takes name

Individually they read really well, but when lined up next to each other it feels inconsistent.
Would this be better if the used the same word stem (e.g. name/names or label/labels)?

Add labels to an issue

Parameters:

{
  owner: "hello",
  repo: "world",
  number: 1,
  labels: ["label/a", "label/b"]
}

Remove label from an issue

Parameters:

{
  owner: "fruit",
  repo: "apple",
  number: 1,
  name: "label/a"
}

I realize that this is based on the GitHub API docs, which show the DELETE endpoint with :name, whereas the POST body takes an input of a bare array (somewhat similar to the inconsistency mentioned in #16).

/cc @bkeepers

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.