pull-requests icon indicating copy to clipboard operation
pull-requests copied to clipboard

[wip] Add binder

Open bollwyvl opened this issue 4 years ago • 8 comments

Binder

  • [x] add a basic "development" binder (we can later add a "pinned" one long-lived branch?)
    • [x] environment.yml: normal stuff, also jupyterlab-tour and nbgitpuller
    • [x] postBuild: pretty much what CI does
    • [x] add "legacy" magic function names for launching under lab 3 with jupyter notebook
  • [x] add anonymous github provider
    • [x] configured by default on binder by copying .binder/jupyter_config.example.json
    • [ ] docs
  • [x] add README badge
    • [ ] update with an "interesting" pre-loaded link to reduce API calls when API limited
  • [ ] VERY WIP config API change
    • [x] just hands around the PRConfig object instead of unpacking it
    • [ ] remove entirely?
  • [ ] testing
    • [x] appear to have broken things

bollwyvl avatar Apr 08 '21 16:04 bollwyvl

Codecov Report

Merging #44 (378432b) into master (909cad7) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #44   +/-   ##
=======================================
  Coverage   61.11%   61.11%           
=======================================
  Files          33       33           
  Lines        1638     1638           
  Branches       94       94           
=======================================
  Hits         1001     1001           
  Misses        632      632           
  Partials        5        5           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 909cad7...378432b. Read the comment docs.

codecov-io avatar Apr 08 '21 16:04 codecov-io

Does what it says on the tin: Screenshot from 2021-04-08 12-31-26

It does pop the nbdime build nag... i'm wondering if we actually need it, or can

  • disable the nbdime labextension (other stuff seems to initialize)
  • disable the build message with tornado cruft

bollwyvl avatar Apr 08 '21 16:04 bollwyvl

Took some doing, but things seem to work up to viewing (locally). Will post a Binder screenshot... probably broke a bunch of stuff, and may need to make some separate PRs (that might be good to land before 3.0 final, perhaps).

bollwyvl avatar Apr 09 '21 15:04 bollwyvl

Cool, github-anonymous works all the way up to viewing text diffs, and correctly doesn't try to paginate across all of jupyterlab's pull requests:

Screenshot from 2021-04-09 12-46-48

I was able to look at about 10 pull requests (and each of their files) before it started dying with 403:

Screenshot from 2021-04-09 12-48-47

bollwyvl avatar Apr 09 '21 16:04 bollwyvl

@fcollonval Haven't gotten around to warming this up, but the approach has legs, and it's worth checking out the binder.

As this PR has gotten rather large (like i do), I think an effective path forward might be to do some refactoring before trying to land this:

Some of those could go on 3.x...

  • normalize the python unit test approaches
    • some pytest.fixtures would drastically reduce some test boilerplate
  • use entry_points for the manager providers
    • this way, someone could add a your-dvcs-provider-here if they are able to test it better (e.g. bitbucket)
  • lean more heavily on the PRConfig object rather than fancy names in the manager constructors
  • introduce some concept of scoped searches
  • investigate some built-in caching (sadly, there is no tornado-cache a la requests-cache, but let's not go to threadtown unless we have to)
    • we could also (optionally) use the tornado curl client, it is generally better, just slightly harder to install on windows

but some of the bigger ones might need to be 4.x...

  • allow multiple, simultaneous providers

bollwyvl avatar Apr 12 '21 13:04 bollwyvl

@fcollonval Haven't gotten around to warming this up, but the approach has legs, and it's worth checking out the binder.

Awesome work @bollwyvl - sorry to not look into this earlier.

As this PR has gotten rather large (like i do), I think an effective path forward might be to do some refactoring before trying to land this

Agreed, I'll help open small PRs.

introduce some concept of scoped searches

vscode GitHub plugin uses the GitHub search API. And the user can customize groups like this:

"githubIssues.queries": [
        {
            "label": "My Issues",
            "query": "default"
        },
        {
            "label": "All Bugs",
            "query": "state:open repo:${owner}/${repository} sort:updated-desc label:type:Bug"
        },
        {
            "label": "All Features",
            "query": "state:open repo:${owner}/${repository} sort:updated-desc label:type:Enhancement"
        }
    ]

We could add a config button in the frontend panel that would open a form allowing the user to customize the search in a similar way. The question being do we want it to be a setting or a state?

  • investigate some built-in caching (sadly, there is no tornado-cache a la requests-cache, but let's not go to threadtown unless we have to)

What do you think of aiocache?

but some of the bigger ones might need to be 4.x...

  • allow multiple, simultaneous providers

Yep this definitely needs redesign of the extension code.

fcollonval avatar Apr 19 '21 06:04 fcollonval

vscode GitHub plugin uses the GitHub search API. And the user can customize groups like this:

anonymous can't use the github search API, so we at least need some approach that is compatible with that, but that's just a subset of the larger problem.

setting or a state?

I'm pretty sure a it should be settings on the frontend, as it would need an "interesting" UI to make it useful, but only the provider knows what the fields are.

Basically, we'll want each search provider to create a JSON schema for "what is a search" that can be mapped to whatever the search provider has, and the provider would have to provide what it knows. So anonymous would only know about:

  • org (a string)
  • repo (a string)
  • state (an enum)

A full github/gitlab/bitbucket/gitea provider knows about the above, and (probably) a ton of other things, plus it could go off and do other queries to provide autocomplete values.

Anyhow, so the client would PUT a body that conforms to that, and then have a search that existed for the duration of the server lifetime. When the server restarts, you'd chunk back into it.

bollwyvl avatar Apr 19 '21 12:04 bollwyvl