Detect proxy in desktop #396

roll · 2024-05-22T08:46:29Z

fixes pip timeout during initialisation in a corporate/public sector environment with HTTP proxy #359

Please make sure that all the checks pass. Please add here any additional information regarding this pull request. It's highly recommended that you link this PR to an issue (please create one if it doesn't exist for this PR)

roll · 2024-05-22T08:49:23Z

@pdelboca
@guergana
Please take a look.

I don't yet have a proper env with proxy to test; going to do so on Monday

cloudflare-workers-and-pages · 2024-05-22T08:52:56Z

Deploying opendataeditor with Cloudflare Pages

Latest commit:	`4ac59f7`
Status:	✅ Deploy successful!
Preview URL:	https://a2b7f170.opendataeditor.pages.dev
Branch Preview URL:	https://359-detect-proxy.opendataeditor.pages.dev

View logs

pdelboca

Looks good @roll .

get-proxy-settings documentation suggest to use native electron support instead: https://www.npmjs.com/package/get-proxy-settings

If you are using electron it is recommended to use electron/chromium built in proxy support which is much more advanced.

Have you tried out the native support? https://github.com/electron/electron/blob/main/docs/api/command-line-switches.md#--proxy-serveraddressport

roll · 2024-05-27T07:23:56Z

@pdelboca
Good finding, I'll try it

roll · 2024-06-04T12:32:37Z

@pdelboca
No, we can't use Electron native detector -- it doesn't provide credentials information (I guess for security reasons). So I guess it's only useful if network calls are made through electron itself.

@pdelboca
@guergana
I tested it, it works for me. Can you please try? I think we might need to make some adjustments later when we see it in the real world to cover hidden edge cases.

guergana · 2024-06-06T16:51:54Z

@pdelboca No, we can't use Electron native detector -- it doesn't provide credentials information (I guess for security reasons). So I guess it's only useful if network calls are made through electron itself.

@pdelboca @guergana I tested it, it works for me. Can you please try? I think we might need to make some adjustments later when we see it in the real world to cover hidden edge cases.

@roll which operating system did you test it with? Since this a desktop issue, I think we should try to test in Ubuntu, Windows 10 (version of the user who reported the issue) and Mac.

roll · 2024-06-10T07:39:25Z

@guergana
Currently, it is tested in Ubuntu.

I think there is a gotcha with the get-proxy-settings library as it will definitely won't work in each and every case. My hope that at least it will cover the most of them. In next iteration, we additionaly might need to add an UI for setting up a proxy I suspect

guergana · 2024-06-10T08:16:22Z

@roll how did you test the proxy? I tried configuring the default proxy and then tried to make the traffic be caught with postman and I couldn't make it work..

Could you list the steps that you followed? I think I missed something. I will also try to test it in my virtual machine for windows.

roll · 2024-06-10T08:21:52Z

@guergana
I enabled a proxy in Ubuntu and then ensured that pip and python calls were done using it in the env vars (in the electron logs)

guergana · 2024-06-21T15:59:03Z

Hello, @roll I am testing the branch in windows 10 pro (with a virtual machine in Ubuntu) and i got the error with the proxy activated:

I am not sure the issue is fixed for Windows 10 (the original operating system of the person who opened the issue).

Steps to reproduce the error in Windows 10:

configure a proxy on windows 10. (i used this address i found online but I am not sure this works really as a proper proxy).
install the app. if you have already installed the app before make sure to delete the .opendataeditor folder before.
after install, the app opens and when the startup popup is showing this message appears.

There is nothing related to this error in the logs c:/user/.opendataeditor/logger/main.log

Note: the port 8080 is not set in the address i tested with, I also tried with :80 which exists but is serving the static files 🙈

I probably need to find a proper proxy address to test with but I still don't understand why we need a proxy upon loading the app, shouldn't all the packages already be contained in the installation package instead of downloading them on startup?

guergana · 2024-06-21T16:37:57Z

desktop/python.ts

@@ -18,22 +19,19 @@ export async function ensurePython() {
  log.info('[ensurePython]', { message })
 }

-export async function ensureLibraries() {
+export async function ensureLibraries(props: { proxyUrls: types.IProxyUrls }) {
  log.info('[ensureLibraries]')

  const required = await readRequiredLibraries()
  const installed = await readInstalledLibraries()
  const missing = required.filter((spec) => !installed.includes(spec))


why would there be libraries missing from the installer?

It uses the concept of idempotency. The user at any point of time can have:

no installed dependencies

partially installed dependencies (if a new version of ODE adds a new dependency)

partially outdated dependencies (if a new version of ODE bumps a dependency's version)

So the installer calculates the diff and applies missing parts

The concept sounds elegant, but this means also that if a user downloads the app and then wants to open it for the first time and it's not connected to the internet, it will also throw the error, right? I think it is confusing why some additional libraries need to be installed if the user has already downloaded the binaries. I would switch to a 100% self contained version, this would also shorten the loading time at startup. @roll

@guergana I would also switch of course =)

roll · 2024-06-24T07:51:24Z

I probably need to find a proper proxy address to test with but I still don't understand why we need a proxy upon loading the app, shouldn't all the packages already be contained in the installation package instead of downloading them on startup?

The problem that packaging the python dependencies with tools like py2exe doesn't really work. We tried a few times and never got any reliable results. Of course, it was pretty time-limited attempts maybe with proper timeline it would be possible to achieve.

The second thing is accessing remote files via the server -- it also requires proxy information. It can be eliminated if fetchFile endpoint is implemented directly in Electron though

guergana · 2024-06-24T08:17:17Z

I probably need to find a proper proxy address to test with but I still don't understand why we need a proxy upon loading the app, shouldn't all the packages already be contained in the installation package instead of downloading them on startup?

The problem that packaging the python dependencies with tools like py2exe doesn't really work. We tried a few times and never got any reliable results. Of course, it was pretty time-limited attempts maybe with proper timeline it would be possible to achieve.

The second thing is accessing remote files via the server -- it also requires proxy information. It can be eliminated if fetchFile endpoint is implemented directly in Electron though

Since we have more developer time available now probably than in the previous iteration maybe we can try to implement the self contained version? @roll . What do you think about this @pdelboca ?

pdelboca · 2024-06-24T14:33:29Z

@guergana I agree with both of you.

When working on #427 I read a little bit about how to package Python applications and as @roll mentioned, sadly all bundling solutions for python do not work as expected.

I do think that we should bundle it since you are right that it doesn't make sense to have errors or need internet connection to open the application. Maybe instead of an executable file we zip/unzip the virtual environment on build/install.

I'll take a look and propose some ideas.

roll · 2024-07-01T08:50:49Z

CLOSED in favour of #446

(parts of this PR can be used to handle #449)

roll added 7 commits May 14, 2024 15:45

Added an implementation plan

b608bc8

Use better name for starting desktop dev

0c002bc

Added get-proxy-settings lib

e3b90cd

Support env in system calls

ec5a4ad

Implemented detectProxyConfiguration

f63c975

Call pip with http_proxy env

0aaecc1

Removed implementation plan

97a9b78

roll requested review from pdelboca and guergana May 22, 2024 08:48

Added a comment

292e12e

pdelboca approved these changes May 23, 2024

View reviewed changes

roll added 8 commits June 3, 2024 13:48

Merge branch 'main' into 359/detect-proxy

c7cfa66

Pass http proxy to the server as well

3b22462

Added TODO

0231858

Fixed proxy detection

6f92412

Removed env from logging

9728f4c

Support https proxy as well

20f4f3e

Removed underlaying library hacking

53677b7

Updated comment

88c31d1

Trigger Build

4ac59f7

guergana reviewed Jun 21, 2024

View reviewed changes

roll mentioned this pull request Jul 1, 2024

Proxies: handle remote calls that needs to be done by frictionless-py #449

Open

roll closed this Jul 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect proxy in desktop #396

Detect proxy in desktop #396

roll commented May 22, 2024

roll commented May 22, 2024

cloudflare-workers-and-pages bot commented May 22, 2024 •

edited

Loading

pdelboca left a comment •

edited

Loading

roll commented May 27, 2024

roll commented Jun 4, 2024

guergana commented Jun 6, 2024 •

edited

Loading

roll commented Jun 10, 2024

guergana commented Jun 10, 2024

roll commented Jun 10, 2024

guergana commented Jun 21, 2024 •

edited

Loading

guergana Jun 21, 2024

roll Jun 24, 2024 •

edited

Loading

guergana Jun 24, 2024

roll Jun 24, 2024

roll commented Jun 24, 2024 •

edited

Loading

guergana commented Jun 24, 2024

pdelboca commented Jun 24, 2024

roll commented Jul 1, 2024

Detect proxy in desktop #396

Detect proxy in desktop #396

Conversation

roll commented May 22, 2024

roll commented May 22, 2024

cloudflare-workers-and-pages bot commented May 22, 2024 • edited Loading

Deploying opendataeditor with Cloudflare Pages

pdelboca left a comment • edited Loading

Choose a reason for hiding this comment

roll commented May 27, 2024

roll commented Jun 4, 2024

guergana commented Jun 6, 2024 • edited Loading

roll commented Jun 10, 2024

guergana commented Jun 10, 2024

roll commented Jun 10, 2024

guergana commented Jun 21, 2024 • edited Loading

guergana Jun 21, 2024

Choose a reason for hiding this comment

roll Jun 24, 2024 • edited Loading

Choose a reason for hiding this comment

guergana Jun 24, 2024

Choose a reason for hiding this comment

roll Jun 24, 2024

Choose a reason for hiding this comment

roll commented Jun 24, 2024 • edited Loading

guergana commented Jun 24, 2024

pdelboca commented Jun 24, 2024

roll commented Jul 1, 2024

cloudflare-workers-and-pages bot commented May 22, 2024 •

edited

Loading

pdelboca left a comment •

edited

Loading

guergana commented Jun 6, 2024 •

edited

Loading

guergana commented Jun 21, 2024 •

edited

Loading

roll Jun 24, 2024 •

edited

Loading

roll commented Jun 24, 2024 •

edited

Loading