-
Notifications
You must be signed in to change notification settings - Fork 27
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
Initialize UserContributedToStream using Graphql #78
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I like that this consumes from a different quota than the REST calls :)
|
||
name = "user_contributed_to" | ||
query_path = "user.repositoriesContributedTo.nodes" | ||
primary_keys = ["username"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to fetch the data by some other, more stable, identifier (thinking nodeId
or id
), as users can change their username at any time? I don't know what consequences this would have on this stream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sadly, I did not find any...
tap_github/user_streams.py
Outdated
""" | ||
|
||
schema = th.PropertiesList( | ||
th.Property("id", th.StringType), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I understand, id
in github's graphql context is the same as nodeId
in REST. Would it be worth renaming the properties here to give a consistent naming in the output of the tap? REST has both id
(an integer) and node_id
(string).
Co-authored-by: Laurent Savaete <laurent@where.tf>
tap_github/client.py
Outdated
https://docs.python-requests.org/en/latest/api/#requests.Response | ||
""" | ||
resp_json = response.json() | ||
data = glom(resp_json["data"], self.query_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL glom
🙂
The SDK already supports JSONPath, so I'm curious what are the benefits of glom over that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@edgarrmondragon I am trying to use JSONPath and it works with online tools but I can't get it to work in the code.
This is what I am doing:
resp_json = response.json()
yield from extract_jsonpath(self.query_jsonpath, input=resp_json)
with query_jsonpath = "$.data.user.repositoriesContributedTo.nodes.*"
The list returns empty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm I think I fixed it in 9435df7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ericboucher looks good!
Kudos, SonarCloud Quality Gate passed!
|
Initialize UserContributedToStream using Graphql
This also fixes #48