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

schema r-client vs api spec #68

Closed
przell opened this issue Jul 22, 2021 · 8 comments
Closed

schema r-client vs api spec #68

przell opened this issue Jul 22, 2021 · 8 comments
Assignees
Labels
bug Something isn't working high
Milestone

Comments

@przell
Copy link
Member

przell commented Jul 22, 2021

@m-mohr, @flahn
I was testing the EODC backend and couldn't get batch jobs to run. I'm using the dev backend (batch is currently not working on live).
The Process Graphs from the R-Client fill the schema part like this:

p = processes()

data = p$load_collection(id = collection, 
                         spatial_extent = bbox,
                         temporal_extent = time_range, 
                         bands = bands)

ndvi_calc = p$reduce_dimension(data = data, 
                               dimension = "bands", 
                               reducer = function(data, context) {
                                 red = data[1]
                                 nir = data[2]
                                 ndvi = (nir-red)/(nir+red)
                                 return(ndvi)
                               })

result = p$save_result(data = ndvi_calc, format="NetCDF")

graph_info = create_user_process(result, id = "test", submit = FALSE)
print(jsonlite::toJSON(graph_info, pretty = TRUE, auto_unbox = TRUE))

{
  "id": "test",
  "process_graph": {
    "load_collection_CXXKP3729P": {
      "process_id": "load_collection",
      "arguments": {
        "id": "boa_sentinel_2",
        "spatial_extent": {
          "west": 11.5383,
          "east": 11.5383,
          "south": 46.4867,
          "north": 46.4867
        },
        "temporal_extent": [
          "2016-01-07T12:00:00Z",
          "2016-04-29T12:00:00Z"
        ],
        "bands": [
          "B04",
          "B08"
        ]
      }
    },
    "reduce_dimension_ZUASW3097D": {
      "process_id": "reduce_dimension",
      "arguments": {
        "data": {
          "from_node": "load_collection_CXXKP3729P"
        },
        "reducer": {
          "process_graph": {
            "array_element_NKODW5210T": {
              "process_id": "array_element",
              "arguments": {
                "data": {
                  "from_parameter": "data"
                },
                "index": 1,
                "return_nodata": true
              }
            },
            "array_element_IJDDF0819R": {
              "process_id": "array_element",
              "arguments": {
                "data": {
                  "from_parameter": "data"
                },
                "index": 0,
                "return_nodata": true
              }
            },
            "subtract_SJQET9903U": {
              "process_id": "subtract",
              "arguments": {
                "x": {
                  "from_node": "array_element_NKODW5210T"
                },
                "y": {
                  "from_node": "array_element_IJDDF0819R"
                }
              }
            },
            "add_EEYVX5550O": {
              "process_id": "add",
              "arguments": {
                "x": {
                  "from_node": "array_element_NKODW5210T"
                },
                "y": {
                  "from_node": "array_element_IJDDF0819R"
                }
              }
            },
            "divide_ODGYJ5214M": {
              "process_id": "divide",
              "arguments": {
                "x": {
                  "from_node": "subtract_SJQET9903U"
                },
                "y": {
                  "from_node": "add_EEYVX5550O"
                }
              },
              "result": true
            }
          }
        },
        "dimension": "bands",
        "context": null
      }
    },
    "save_result_MVCCL0551V": {
      "process_id": "save_result",
      "arguments": {
        "data": {
          "from_node": "reduce_dimension_ZUASW3097D"
        },
        "format": "NetCDF",
        "options": [
          "{}"
        ]
      },
      "result": true
    }
  },
  "parameters": [],
  "returns": {
    "schema": {
      "type": "boolean",
      "subtype": [],
      "pattern": [],
      "parameters": [],
      "items": {
        "type": {}
      },
      "minItems": [],
      "maxItems": []
    }
  }
} 

The error the I get from EODC is this:

> job = create_job(graph = result,
+                  title = out_name,
+                  description = out_name,
+                  format = "netCDF", 
+                  con = con)
SERVER-ERROR: {'returns': {'schema': {0: {'minItems': ['Not a valid number.'], 'subtype': ['Not a valid string.'], 'pattern': ['Not a valid string.'], 'maxItems': ['Not a valid number.']}}}, 'id': ['Field may not be null.']}

@sophieherrmann pointed out that the API Spec asks some parameters to be strings:
https://api.openeo.org/#operation/create-job

On other backends (VITO, EURAC) I don't get this error.

@m-mohr
Copy link
Member

m-mohr commented Jul 22, 2021

Yes, the defaults provided by the R client are counter-productive here as they don't comply with the API spec. The R client should not provide the fields that don't contain data, e.g. subtype, pattern, minItems etc. The other back-ends don't strictly check the schemas it seems so they are more tolerant, but that doesn't make the EODC back-end invalid. The only thing that I don't understand is "'id': ['Field may not be null.']}". That could be an issue on EODCs side, but I'm not exactly sure which id that refers to.

@sophieherrmann
Copy link

I'm not sure why @przell gets 'id': ['Field my not be null']. I just tested the job again, and got only the error regarding the return schema fields. Could you check this again, maybe this is already solved by one of the updates I deployed.

@m-mohr
Copy link
Member

m-mohr commented Sep 30, 2021

We should try to not send default values to avoid interoperability issues, but overall this is a back-end issue on EODCs side.

@m-mohr m-mohr added the low label Sep 30, 2021
@sophieherrmann
Copy link

Maybe I'm missing something, but if I understand it correctly EODC is just more strict in respect to default values than other back-ends, but we just enforce what is defined in the API. So I wouldn't say this issue is on EODC side but rather on r-client side, or?

Considering that the 'id': ['Field may not be null'] as fixed some time ago.

@m-mohr
Copy link
Member

m-mohr commented Oct 5, 2021

Having a closer look and with the id issue being resolved, I think the issue is indeed on the R client side as it provides empty arrays for a couple of value that don't accept arrays:

For example:

      "subtype": [],
      "pattern": [],
      "items": {
        "type": {}
      },
      "minItems": [],
      "maxItems": []

It could still be a EODC issue depending on how the validation is implemented, but we need to try it again after the array issues have been resolved in the R client.

@m-mohr m-mohr added bug Something isn't working medium and removed low labels Oct 5, 2021
@m-mohr m-mohr added this to the v1.1.0 / CRAN milestone Oct 7, 2021
@m-mohr m-mohr added high and removed medium labels Oct 7, 2021
@flahn flahn assigned przell and unassigned flahn Oct 21, 2021
@flahn
Copy link
Member

flahn commented Oct 21, 2021

@przell can you test this? The changes are in the develop branch.

@m-mohr
Copy link
Member

m-mohr commented Oct 21, 2021

Works for me.
Haven't tested what happens if you add a parameter though.

@m-mohr
Copy link
Member

m-mohr commented Oct 22, 2021

@flahn Tested again. I've tested adding a parameter/variable and that works very well, but I found a minor issue with the return value: It has an "optional" flag, which doesn't make sense (or at least is not supported) by the openEO API.
You could also not export the optional flag if the default value false is given, but that's not a real issue for me.

The code is:

library(openeo)

con = connect("https://openeo.eodc.eu")

p = processes()

cid = create_variable("test", "A test!", "string", "collection-id")

data = p$load_collection(id = cid, 
                                                spatial_extent = list(west=16.06, 
                                               south=48.06,
                                               east=16.65,
                                               north=48.35),
                         temporal_extent = c("2017-03-01", "2017-06-01"))

ndvi_calc = p$reduce_dimension(data = data, 
                               dimension = "bands", 
                               reducer = function(data, context) {
                                 red = data[1]
                                 nir = data[2]
                                 ndvi = (nir-red)/(nir+red)
                                 return(ndvi)
                               })

result = p$save_result(data = ndvi_calc, format="NetCDF")

as(result, "Process")

Result is:

{
  "process_graph": {
    "load_collection_FFLLR9155Z": {
      "process_id": "load_collection",
      "arguments": {
        "id": {
          "from_parameter": "test"
        },
        "spatial_extent": {
          "west": 16.06,
          "south": 48.06,
          "east": 16.65,
          "north": 48.35
        },
        "temporal_extent": [
          "2017-03-01",
          "2017-06-01"
        ]
      }
    },
    "reduce_dimension_OGHLQ7860K": {
      "process_id": "reduce_dimension",
      "arguments": {
        "data": {
          "from_node": "load_collection_FFLLR9155Z"
        },
        "reducer": {
          "process_graph": {
            "array_element_GTWFZ9765Y": {
              "process_id": "array_element",
              "arguments": {
                "data": {
                  "from_parameter": "data"
                },
                "index": 1,
                "return_nodata": true
              }
            },
            "array_element_KUJTN1177N": {
              "process_id": "array_element",
              "arguments": {
                "data": {
                  "from_parameter": "data"
                },
                "index": 0,
                "return_nodata": true
              }
            },
            "subtract_ZCLSY7797Y": {
              "process_id": "subtract",
              "arguments": {
                "x": {
                  "from_node": "array_element_GTWFZ9765Y"
                },
                "y": {
                  "from_node": "array_element_KUJTN1177N"
                }
              }
            },
            "add_ZEOVF9609G": {
              "process_id": "add",
              "arguments": {
                "x": {
                  "from_node": "array_element_GTWFZ9765Y"
                },
                "y": {
                  "from_node": "array_element_KUJTN1177N"
                }
              }
            },
            "divide_PIWMV6962Z": {
              "process_id": "divide",
              "arguments": {
                "x": {
                  "from_node": "subtract_ZCLSY7797Y"
                },
                "y": {
                  "from_node": "add_ZEOVF9609G"
                }
              },
              "result": true
            }
          }
        },
        "dimension": "bands",
        "context": null
      }
    },
    "save_result_EMPAY7817J": {
      "process_id": "save_result",
      "arguments": {
        "data": {
          "from_node": "reduce_dimension_OGHLQ7860K"
        },
        "format": "NetCDF",
        "options": {}
      },
      "result": true
    }
  },
  "parameters": [
    {
      "name": "test",
      "description": "A test!",
      "optional": false,
      "schema": {
        "type": "string",
        "subtype": "collection-id",
        "pattern": "^[\\w\\-\\.~/]+$"
      }
    }
  ],
  "returns": {
    "optional": false, <!-- here
    "schema": {
      "type": "boolean"
    }
  }
} 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high
Projects
None yet
Development

No branches or pull requests

4 participants