#42 Creating Gitcoin Endpoints #49
Conversation
❌ Deploy Preview for whimsical-cuchufli-3a5565 failed.
|
|
okay @thelastjosh just got a chance to sit down with this! sorry for the delay, but ready for a round of review now. there's a few TODOs that i tossed in there to get some of your input on. i'll leave my own comments to start some conversation! one general question i had - how can i run this api server locally & test these endpoints? i'm thinking i'd just use postman to try a few queries out. i don't see any place to |
| import fetch from 'node-fetch' | ||
|
|
||
| function apiRequest(path: string, method: string, data: any) { | ||
| return fetch(path, { | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| method, | ||
| redirect: 'follow', | ||
| body: JSON.stringify(data), | ||
| }).then((res) => res.json()) | ||
| } |
There was a problem hiding this comment.
i can revert this if y'all want, so i'm not touching the other existing endpoints - should probably be done in a separate refactoring PR 😄
There was a problem hiding this comment.
i saw this function was getting reused, so i figured pulling it out to be imported would be better. let me know if this should go somewhere else though!
There was a problem hiding this comment.
looks good to me, just wanted to flag this for you @ipatka since the function affects the aave repo
| @@ -0,0 +1,13 @@ | |||
| import fetch from 'node-fetch'; | |||
| import { HttpMethod } from './config'; | |||
There was a problem hiding this comment.
todo: should use the library that other files are importing
| export enum HttpMethod { | ||
| POST='POST', | ||
| } |
There was a problem hiding this comment.
todo: this should use a library rather than being our own custom enum
There was a problem hiding this comment.
i should maybe revert this change, but i do see some npm audit fixs we should do
| @@ -16,6 +16,7 @@ export const handler: APIGatewayProxyHandlerV2 = async (event) => { | |||
| const eventId = event?.pathParameters?.id; | |||
| if (!eventId) return { statusCode: 400 }; | |||
|
|
|||
| // TODO: this could be pulled out to a generic file. | |||
There was a problem hiding this comment.
i'd definitely like to pull out as many of these types as we can - especially makes it easier for someone writing their own endpoints, to be able to fill in a few specificities, rather than having to write the entirety of these functions
My impression is that this should be straightforward supposing that the endpoints are up on The Graph. @ipatka ? |
@thelastjosh @ipatka ideally, there should be a "local-only" development version that i can run, but i don't see how to do that. |
|
Hi @DrewMcArthur, |
|
@Rashmi-278 yeah would love to! i've messaged you on slack. |
closes #42