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.

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

Include options to EO-RIVER tools for requesting data-url only and to not only take the largest polygon #40

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jurjendejong
Copy link

The changes are untested as I do not know how I could...

@@ -800,14 +802,16 @@ def get_water_network():

j = request.json

use_url = j['use_url']
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make it more explicit, for example:

"output_type": ['geojson', 'geojson_url']

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My implementation is already applied in other places of the code, and thus makes things more uniform. I agree that your approach is more nice, but it lacks consistency.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistency with what? This approach is more consistent since you won't need to add another parameter if we add export tot GDrive, storage buckets, etc.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok, it was already like that :)

@gena
Copy link
Collaborator

gena commented Aug 6, 2020

The build is failing, should be fixed before merge

@jurjendejong
Copy link
Author

The build is failing, should be fixed before merge

Are you able to run the CI? I do not have the private_key_file.

@jurjendejong
Copy link
Author

jurjendejong commented Aug 7, 2020

I managed to get the private_key working, both local and on my own TRAVIS (see below). Thanks for the help! This made me do small adjustments. However, still it's failing because I cannot setup the private_key at the openearth-TRAVIS (no permission I suppose).

Here is my prove that build is succesful: https://travis-ci.com/github/jurjendejong/hydro-engine-service/builds/178883887

Edit: I realise there is no need to hurry as I can also now use the local build. Le's get back to this after your holiday.

@SiggyF
Copy link
Contributor

SiggyF commented Mar 30, 2021

Can we update this to the latest structure and merge it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants