Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: Make subgraph URL's configurable #2105

Closed
ahhda opened this issue Dec 1, 2023 · 3 comments · Fixed by #2126 or #2381
Closed

feat: Make subgraph URL's configurable #2105

ahhda opened this issue Dec 1, 2023 · 3 comments · Fixed by #2126 or #2381
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@ahhda
Copy link
Contributor

ahhda commented Dec 1, 2023 •
edited by fleupold
Loading

Problem

We use two subgraphs in our backend, Uniswap v3 and Balancer v2. The Base URL for these subgraphs has been made configurable in #2126, however it turns out that in order to use third party hosted solutions, we not only need the base URL but instead the entire URL configurable.

For example the Alchemy URL is in the format https://subgraph.satsuma-prod.com/API_KEY/ACCOUNT/balancer-v2-subgraph/version/v0.0.1-new-version/api.

This requires a code change so that the URL's can be configured via env variables/infra config.

Suggested solution

Make entire subgraph URL's configurable similar to what we have for ethRpc or ethplorerApiUrl

Additional context

As the hosted service is being shut down, the hosted subgraphs that we use currently have performance issues. We are now trying to host the subgraphs on either the graph explorer or Alchemy.

This should help fix #1759, #1020

Acceptance criteria

  • Entire subgraph URL for Uniswap and Balancer are configurable via infra config
@ahhda ahhda added the good first issue Good for newcomers label Dec 1, 2023
@ssvarmma
Copy link

ssvarmma commented Dec 1, 2023

amazing

@fleupold
Copy link
Contributor

fleupold commented Dec 4, 2023

Assigning to @narcis96 as a good first issue (pending discussion with @sunce86 on other options for the fee feature)

@narcis96 narcis96 mentioned this issue Dec 6, 2023
Merged
narcis96 added a commit that referenced this issue Dec 11, 2023
@narcis96
Make subgraph URL's configurable (#2126)
bf1d4a7 
# Description
<!--- Describe your changes to provide context for reviewers, including
why it is needed -->
This PR aims to fix
[2105](#2105)

# Changes
The subgraph base url is read from the environment variables.

## How to test
All unit tests still pass

Co-authored-by: narcis <[email protected]>
@ahhda ahhda reopened this Jan 8, 2024
@fleupold
Copy link
Contributor

The issue was re-opened, because we actually need to make the full URL (not just the base) configurable for each liquidity type.

@fleupold fleupold added the help wanted Extra attention is needed label Jan 16, 2024
@fredmarques fredmarques mentioned this issue Feb 6, 2024
Merged
MartinquaXD added a commit that referenced this issue Mar 22, 2024
@fredmarques @MartinquaXD @fleupold
feat: change subgraph constructor to use full urls (#2381)
6ff4eda 
# Description
This PR fixes #2105 

# Changes
Subgraph constrictor now uses full urls

## How to test
All unit tests still pass

## Related Issues

Fixes  #2105

---------

Co-authored-by: Martin Beckmann <[email protected]>
Co-authored-by: Felix Leupold <[email protected]>
mfw78 pushed a commit that referenced this issue Mar 22, 2024
@fredmarques @MartinquaXD
@fleupold @mfw78
feat: change subgraph constructor to use full urls (#2381)
2768a99 
# Description
This PR fixes #2105 

# Changes
Subgraph constrictor now uses full urls

## How to test
All unit tests still pass

## Related Issues

Fixes  #2105

---------

Co-authored-by: Martin Beckmann <[email protected]>
Co-authored-by: Felix Leupold <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
4 participants
@fleupold @narcis96 @ahhda @ssvarmma