openforis / sepal

Geographical Data Processing in the Cloud
https://sepal.io/
MIT License
207 stars 46 forks source link

Earth Engine is not working on Venvs #134

Closed dfguerrerom closed 2 years ago

dfguerrerom commented 2 years ago

Describe the bug I'm testing the modules envs and it seems like there is a problem when initialize Earth Engine.

To Reproduce I have tested it in two different environments and this is happening. image

12rambau commented 2 years ago

This is actually leading apps based on GEE to fail. when sepal-ui finds out that ee is not Initialized it tries to use my GEE accounts as a service account using an encrypted json included in the lib. This behaviour has been set-up for deploying documentation and testing purposes. Users should never use my GEE account so the apps are simply crashing as the env var EE_PRIVATE_KEYis not set.

Could it be possible to init GEE in all the venv the same way it's done for instances ? The main issue I already see is that app that does not require GEE havn't the earthengine-api installed

cdanielw commented 2 years ago

We're actually using a forked version of earthengine-api as a system dependency. If the same fork is used within the venvs, things should be OK. Here's the requirement: git+git://github.com/openforis/earthengine-api.git@v0.1.270#egg=earthengine-api&subdirectory=python

Maybe we can find some way around needing this fork at some point.

12rambau commented 2 years ago

git+git://github.com/openforis/earthengine-api.git@v0.1.270#egg=earthengine-api&subdirectory=python

Is there any reason why you specified the egg folder name and subdirectory ?

cdanielw commented 2 years ago

In the past, we used the python version inside the main server. But we didn't manage to make it multi-user (though later on I've seen some options on how to do that) That's the reason for the fork to begin with. Now, it's only needed for the credentials. We're generating the file earthengine-api expects:

~/.config/earthengine/credentials

But we don't include the refresh_token there. Instead, we rely on SEPAL to deal with refreshing the access_token, and update the credentials file. We can in theory include therefresh_token, to make the earthengine-api happy. But that would require to have the credentials file writable by the user, which I'm a bit hesitant about.

cdanielw commented 2 years ago

git+git://github.com/openforis/earthengine-api.git@v0.1.270#egg=earthengine-api&subdirectory=python

Is there any reason why you specified the egg folder name and subdirectory ?

Not in particular. That's the way I got things working, and I stopped at that point.

12rambau commented 2 years ago

But that would require to have the credentials file writable by the user, which I'm a bit hesitant about.

I'm always reluctant to let end user modify these files


Not in particular. That's the way I got things working, and I stopped at that point.

I'll check my comprehension of pip egg and subdirectory keywords but from what I recall the one you set are the default one (egg name default is lib name which is here earthengine-api/) and default subdirectory is obviously where all python libs live -> python/

12rambau commented 2 years ago

ok subdiretory is compulsory because the python setup.py file is not at the root of the repo. egg may be excluded. I'll test it from my end and let you know !

https://stackoverflow.com/a/49768354/6734243

12rambau commented 2 years ago

The next release of sepal_ui will be based on the OpenForis fork of earthengine-apiand the build have passed. I'll close it once it's tested on an app

cdanielw commented 2 years ago

Can we close this issue?

12rambau commented 2 years ago

not yet, unfortunately, it will be closed with 2.5.0

12rambau commented 2 years ago

Fixed by https://github.com/openforis/sepal/commit/397af40adb88d2d452f6ff7d92513e17f52cca05

12rambau commented 2 years ago

False joy... we ended up with a pipy error :

Collecting sepal-ui==2.5.0
  Downloading sepal-ui-2.5.0.tar.gz (778 kB)
  Preparing metadata (setup.py): started
  Preparing metadata (setup.py): finished with status 'done'
ERROR: Packages installed from PyPI cannot depend on packages which are not also hosted on PyPI.
sepal-ui depends on earthengine-api@ git+git://github.com/openforis/earthengine-api.git@v0.1.270#egg=earthengine-api&subdirectory=python

sepal_ui 2.5.1 revert to the original earthengine API.

I changed the requirements.txt of the GFC wrapper (https://github.com/openforis/gfc_wrapper_python/commit/5eab81061ebeec032b8d7dd536eb5cb90146e13a) to test if it's sufficent. let's see in the next built.

12rambau commented 2 years ago

It works.

So the final solution is the following: