Allow to compose env variables in HTTP request #2616

oliviertassinari · 2023-09-02T14:00:47Z

Summary 💡

I would like to use my process.env.GITHUB_TOKEN token, so I wired the value:

However, I need to have this logic:

Authorization: `Bearer ${process.env.GITHUB_TOKEN}`,

for the token to be correctly recognized. I would rather not add Bearer to GITHUB_TOKEN because I'm also using https://octokit.github.io/rest.js/v20#authentication.

Motivation

Without tokens, GitHub API is rate-limited at 10 requests per minute. With the token, the limit is x83 higher.

For unauthenticated requests, the rate limit allows for up to 60 requests per hour.

User access token requests are limited to 5,000 requests per hour

https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#rate-limits-for-requests-from-personal-accounts

Versions

v0.1.26

The text was updated successfully, but these errors were encountered:

Janpot · 2023-09-04T10:32:36Z

One solution could be to allow users to bind parameters, not only to UI state, but also to environment variables. That way we could have the best of both worlds:

Explicit declaration of which parameters are in use by our queries
Full freedom in how to compose them inside of the different query properties

apedroferreira · 2023-09-04T17:21:58Z

One solution could be to allow users to bind parameters, not only to UI state, but also to environment variables.

So I guess that we could allow the user to bind to an expression in these cases, and in these cases that expression would have access to the values of the environment variables. We could try something like that?

JerryWu1234 · 2023-09-05T02:15:30Z

I want to learn more about this. Which button can open this dialog?

Janpot · 2023-09-05T08:11:11Z

So I guess that we could allow the user to bind to an expression in these cases, and in these cases that expression would have access to the values of the environment variables. We could try something like that?

Just to be sure I expressed myself clearly. What I meant was to enable binding to an environment variable in the parameter bindings:

So that in the query bindings one could use it like

It's just a proposal, maybe it's still too convoluted.

What I like about it:

it is explicit and lintable. We can warn on app startup when a variable is not supplied.
it centralizes all inputs for a query.
it'll be easy to extend with e.g. cookie support.
more options to compose env variables in bindings. now you can only bind the whole variable, with this new setup you can derive other values from them. (e.g. btoa(`${parameters.user}:${parameters.password}`);)

What I don't like

It adds extra steps before being able to bind to a secret.

@JerryWu1234 You can find it when opening the binding editor for a header:

We're still trying to figure out how to generalize this pattern. I've also had questions about binding to cookies in this dialog as well.

If you have any suggestions on how we could improve discoverability of this feature, or improvements to the documentation around this, I'd gladly hear it or take PRs.

apedroferreira · 2023-09-05T15:17:35Z

Just to be sure I expressed myself clearly. What I meant was to enable binding to an environment variable in the parameter bindings:

Got it, this makes sense!
It does provide the advantages you mentioned and should work well with the environment bindings we already implemented.
The main trade-off might be the extra step instead of being able to just write an expression directly.
But maybe there are also ways that we could make this extra step obvious enough by improving the UX, so that it would be easy for users to see that they could bind parameters to env variables, and that that would be the right place to use them.
I think it's worth a try!

JerryWu1234 · 2023-09-16T09:23:35Z

@apedroferreira
are you trying this issue right now ?

apedroferreira · 2023-09-16T21:47:54Z

@apedroferreira
are you trying this issue right now ?

I might work on it soon or maybe someone else can, but the team is also still going to discuss this to try to define a solution better.

JerryWu1234 · 2023-09-17T00:42:43Z

will write a ideas that the latest solutions if you discuss over?

apedroferreira · 2023-09-17T01:07:29Z

will write a ideas that the latest solutions if you discuss over?

Ok, will do that and then you can take a look at this issue if we decide that it makes sense! :)

IgnisDa · 2023-09-23T09:17:28Z

Hello, I would like this feature. Any update on it?

IgnisDa · 2023-09-23T10:01:00Z

For this time being, I am using this bash script:

#!/usr/bin/env bash

if [ -z "$BASE_URL" ]; then
  echo '$BASE_URL is not set'
  exit 1
fi

find toolpad/pages -name page.yml -exec sed -i "s|http://localhost:8000|$BASE_URL|g" {} \;

yarn toolpad build

rm -rf dist
cd ..
mkdir dist
cp -r ./admin/. ../yarn.lock ./dist/
mv ./dist ./admin/
cd admin

git checkout -- toolpad/pages

JerryWu1234 · 2023-09-25T07:32:46Z

as well as you to wait for the final solution.

Janpot · 2023-09-25T08:00:26Z

@IgnisDa Would the solution described in #2616 (comment) be sufficient to implement your use-case without extra scripts?

IgnisDa · 2023-09-25T08:38:48Z

@Janpot Yeah looks like it should be.

apedroferreira · 2023-09-26T18:38:28Z

Hi @JerryWu1234 , feel free to take this issue and follow the solution proposed in #2616 (comment). The team has agreed that it's a good solution to start with!

So let's show the "environment variables" tab when setting bindings on query parameters.
There is an envVarNames property that can be passed to the ParametersEditor component in order to show this tab, as you can see in /toolpad-app/src/toolpadDataSources/rest/client.tsx.

--- a/packages/toolpad-app/src/toolpadDataSources/rest/client.tsx
+++ b/packages/toolpad-app/src/toolpadDataSources/rest/client.tsx
@@ -531,6 +531,7 @@ function QueryEditor({
                 globalScopeMeta={globalScopeMeta}
                 liveValue={paramsEditorLiveValue}
                 jsRuntime={jsBrowserRuntime}
+                envVarNames={envVarNames}
               />
             </Box>
           </Panel>

The main issue that you will notice is that there will be an error when trying to bind a parameter to an environment variable: Env variables are not supported in this context. This is because right now all parameters are using the jsBrowserRuntime, which cannot access environment variables. Parameters need to use jsServerRuntime for that.
So we need to make sure that jsServerRuntime is always used when a binding is using an environment variable - that’s the main challenge here.
Fell free to try to come up with a good solution for this problem!

It would be great if we could cover these changes in a test too.

Also additionally, we could allow for doing this in Custom function queries, besides HTTP request queries, but that’s optional (can be done separately if it’s not easy).

Let me know if you have any questions and I'll always try to assist and offer feedback.

JerryWu1234 · 2023-09-27T02:13:42Z

Now there are three pr that it's going on.
I will start after I've finished what I'm working on.
thank for your replying.

JerryWu1234 · 2023-09-27T09:58:01Z

@apedroferreira
can I ask a question ?
Is the env variable defined by user was automatically written to process.env?

apedroferreira · 2023-09-27T13:53:30Z

Now there are three pr that it's going on. I will start after I've finished what I'm working on. thank for your replying.

Got it! If you ever feel like you have too much going on or if you don't want to take this issue just let me know, I just tagged you because you seemed interested :)

Is the env variable defined by user was automatically written to process.env?

The environment variables that show as options for bindings are read from the .env file of the project, in the user's local file system.
And yes, those environment variables set in the .env file are automatically available in process.env (that's how dotenv works https://www.npmjs.com/package/dotenv), and process.env is where we are reading them from to get the values server side.

JerryWu1234 · 2023-09-27T14:16:19Z

Got it! If you ever feel like you have too much going on or if you don't want to take this issue just let me know, I just tagged you because you seemed interested :)

yes I'm interested I just tell you my plan, and I will take it

JerryWu1234 · 2023-10-07T13:20:49Z

@apedroferreira

Recently I celebrated a national holiday, But I already began to address this problem.
and I dived deeply into this issue.

However. Can I pass the environment variables to the front-end as a property within process.env?

such as

define: {
    'process.env.TOOLPAD: JSON.stringify(env),
  },

Otherwise, I will pursue an alternative approach to improve jsServerRuntime

JerryWu1234 · 2023-10-09T08:54:47Z

JerryWu1234 · 2023-10-10T05:52:09Z

@apedroferreira @Janpot

I did two ways for this PR.

because now all of the variables are handled in the backend, it will be hard for fixing #2686 if there is any variable in parameter.

oliviertassinari added new feature New feature or request status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 2, 2023

oliviertassinari mentioned this issue Sep 2, 2023

Client-side API requests #1196

Open

oliviertassinari removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 4, 2023

prakhargupta1 assigned apedroferreira Sep 26, 2023

apedroferreira added good first issue Great for first contributions. Enable to learn the contribution process. and removed good first issue Great for first contributions. Enable to learn the contribution process. labels Sep 26, 2023

This was referenced Oct 10, 2023

Allow binding query parameters to environment variables #2777

Merged

a new feature that let user add env variable. #2767

Closed

apedroferreira closed this as completed Nov 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to compose env variables in HTTP request #2616

Allow to compose env variables in HTTP request #2616

oliviertassinari commented Sep 2, 2023 •

edited

Loading

Janpot commented Sep 4, 2023

apedroferreira commented Sep 4, 2023

JerryWu1234 commented Sep 5, 2023

Janpot commented Sep 5, 2023 •

edited

Loading

apedroferreira commented Sep 5, 2023

JerryWu1234 commented Sep 16, 2023

apedroferreira commented Sep 16, 2023 •

edited

Loading

JerryWu1234 commented Sep 17, 2023

apedroferreira commented Sep 17, 2023

IgnisDa commented Sep 23, 2023

IgnisDa commented Sep 23, 2023

JerryWu1234 commented Sep 25, 2023

Janpot commented Sep 25, 2023

IgnisDa commented Sep 25, 2023

apedroferreira commented Sep 26, 2023

JerryWu1234 commented Sep 27, 2023

JerryWu1234 commented Sep 27, 2023

apedroferreira commented Sep 27, 2023

JerryWu1234 commented Sep 27, 2023

JerryWu1234 commented Oct 7, 2023

JerryWu1234 commented Oct 9, 2023

JerryWu1234 commented Oct 10, 2023

Allow to compose env variables in HTTP request #2616

Allow to compose env variables in HTTP request #2616

Comments

oliviertassinari commented Sep 2, 2023 • edited Loading

Summary 💡

Motivation

Versions

Janpot commented Sep 4, 2023

apedroferreira commented Sep 4, 2023

JerryWu1234 commented Sep 5, 2023

Janpot commented Sep 5, 2023 • edited Loading

apedroferreira commented Sep 5, 2023

JerryWu1234 commented Sep 16, 2023

apedroferreira commented Sep 16, 2023 • edited Loading

JerryWu1234 commented Sep 17, 2023

apedroferreira commented Sep 17, 2023

IgnisDa commented Sep 23, 2023

IgnisDa commented Sep 23, 2023

JerryWu1234 commented Sep 25, 2023

Janpot commented Sep 25, 2023

IgnisDa commented Sep 25, 2023

apedroferreira commented Sep 26, 2023

JerryWu1234 commented Sep 27, 2023

JerryWu1234 commented Sep 27, 2023

apedroferreira commented Sep 27, 2023

JerryWu1234 commented Sep 27, 2023

JerryWu1234 commented Oct 7, 2023

JerryWu1234 commented Oct 9, 2023

JerryWu1234 commented Oct 10, 2023

oliviertassinari commented Sep 2, 2023 •

edited

Loading

Janpot commented Sep 5, 2023 •

edited

Loading

apedroferreira commented Sep 16, 2023 •

edited

Loading