scrape.js: Use GraphQL API #148
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
li-boxuan
force-pushed
the
GraphQL
branch
3 times, most recently
from
83f8e67
to
3ec6e81
Compare
|
I notice that JBoss has two linked on https://gci-leaders.netlify.com/ , but none on https://deploy-preview-148--gci-leaders.netlify.com/ . |
blazeu
previously requested changes
Apr 17, 2018
lib/scrape.js
Outdated
| @@ -9,17 +9,16 @@ const wdk = require('wikidata-sdk') | |||
| const cheerio = require('cheerio') | |||
|
|
|||
| const { GITHUB_REPO_INFO_QUERY } = require('./queries') | |||
| const { GITHUB_SEARCH_ORG_QUERY } = require('./queries') | |||
| const { GITHUB_USER_INFO_QUERY } = require('./queries') | |||
There was a problem hiding this comment.
Just do multiple variable on a single destructuring line { a, b, c }
|
@jayvdb I just investigated that problem (JBoss has no link). This is not due to this PR; I use the previous HEAD to build and also found no link on JBoss part. This is because of the name of their gitter room: this link https://gitter.im/jboss-outreach/gci (See #258, which makes use of that link to extract their org name). However, their org name should be I added the following code around line #272 and ran it on my machine: JBoss would now again have two links. if (orgFromWebsites === 'jboss-outreach') {
return 'JBossOutreach'
}Do you have some advice what I should do to fix this problem? Visit every link and see if it would jump to some other link? Or add ad-hoc support for |
|
fwiw, a part of the problem was that 92a68a2 cleared out all of the data in the deploy. We dont need a special case for JBoss. The link in GCI is old, but a head request will give us the new URL. That is needed for any github/gitter URL which changes. |
bekicot
reviewed
Jun 16, 2018
li-boxuan
force-pushed
the
GraphQL
branch
2 times, most recently
from
92c25b3
to
8c704e9
Compare
|
Lots of "Cannot fetch org x from GitHub, error message: unknown error" in the log https://travis-ci.org/coala/gci-leaders/builds/447609704 |
jayvdb
reviewed
Oct 29, 2018
lib/queries/github_user_info.graphql
Outdated
| @@ -0,0 +1,6 @@ | |||
| query ($user: String!) { | |||
| user(login: $user) { | |||
There was a problem hiding this comment.
our other graphql file uses two space indent.
if you think four is better, also create an issue to re-indent the existing file, and we'll make it a newcomer task
There was a problem hiding this comment.
I should have used two space indent. Blame my editor :P
lib/scrape.js
Outdated
| @@ -53,12 +54,6 @@ const CHAT_IMAGES = { | |||
| OTHER: 'images/chat.png', | |||
| } | |||
|
|
|||
| const GH_API_OPTIONS = { | |||
| headers: process.env.GITHUB_TOKEN | |||
| ? { Authorization: `token ${process.env.GITHUB_TOKEN}` } | |||
There was a problem hiding this comment.
keep this, and keep GH_API_BASE, as I am sure we will want to access some features are not available via Graphql, and we dont want people to need to re-invent this, and possibly pick a new token variable name, etc.
Add a note that GH_API_OPTIONS is unused.
andrewda
suggested changes
Oct 29, 2018
lib/queries/github_user_info.graphql
Outdated
| @@ -0,0 +1,6 @@ | |||
| query ($user: String!) { | |||
There was a problem hiding this comment.
Remove space before (
(or add a space on the other ones, I'm not 100% sure which is preferred in GraphQL, but I think I've seen no space used more frequently)
Suggested change
| query ($user: String!) { | |
| query($user: String!) { |
| @@ -0,0 +1,9 @@ | |||
| query($query: String!) { | |||
| search(type:USER, query:$query, first:1) { | |||
There was a problem hiding this comment.
Add space after colons:
Suggested change
| search(type:USER, query:$query, first:1) { | |
| search(type: USER, query: $query, first: 1) { |
Weird, it works fine on my local machine. The log should be sth as follows: |
|
IIRC the number of unauthenticated GraphQL requests you get is lower than normal API hits. Since this is a PR, I think Travis removes the |
Travis tests have failedHey @li-boxuan, 1st Buildcoala --non-interactive -VTravisBuddy Request Identifier: cf361110-db4a-11e8-99b4-75805dd182ba |
jayvdb
requested changes
Oct 29, 2018
bekicot
reviewed
Oct 29, 2018
| return items || [] | ||
| let data, errors | ||
| try { | ||
| ;({ data, errors } = await client.query(GITHUB_SEARCH_ORG_QUERY, { query })) |
There was a problem hiding this comment.
Interesting, what is the semicolon for?
There was a problem hiding this comment.
Not sure, but eslint will be unhappy with that ;)
There was a problem hiding this comment.
I guess it is because you put parentheses in
;({ data, errors } = await client.query(GITHUB_SEARCH_ORG_QUERY, { query }))
Another question, why is the parentheses used?
There was a problem hiding this comment.
Yes, this is because I put parentheses.
I have to use parentheses otherwise there would be a parsing error.
Use GitHub GraphQL API instead of REST API Closes coala#111
jayvdb
approved these changes
Oct 30, 2018
|
ack 8da2685 |
|
@gitmate-bot ff |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use GitHub GraphQL API instead of REST API
closes #111