As an Open Data Hub manager I want the possibility to add new Dataset without getting a validation error, to extend the dataset of the Open Data Hub.

[sseppi]

Testing "Add a record" functionality for the metadata API I got a validation Error while trying to save the new dataset.

As you can see in the screenshot below I get the following error message:

"Required property 'Id' not found in JSON. Path '', line 1, position 878."

The problem seems to be in the ID field that is read only and isn't generated by the API.

[gappc]

@sseppi that seems to be a problem with the ODH, it shouldn't be necessary to provide an ID on dataset creation because it is almost never a good idea to let a user define IDs. @RudiThoeni what is your opinion on this?

[RudiThoeni]

@sseppi, @gappc In the Metadata api i used our long time ago predefined IDs style like.... it.bz.opendatahub.accommodation.lts.room it.bz.opendatahub.article.idm

If we want to have this Type of IDs, maybe i can autogenerate it with at example "com.opendatahub." + odhtype + source For having this i have to set odhtype and source as required.....

But there are cases where this logic does not work at example on it.bz.opendatahub.article we have the dataset for All articles with all sources.....

Another option would be to switch to Autogenerated Ids (GUIDS,UUIDS) or To let the ID be defined by the Editor (In the case of the databrowser i think only a few expert users will edit....)

[gappc]

@RudiThoeni in my opinion the values that are now stored in the Id field should be put into another field (e.g. ApiId), the Id field should be managed by ODH only (a user should not be allowed to define technical IDs).

[RudiThoeni]

could be an option, i will have a look at the metadata api this sprint

[RudiThoeni]

I used the field ApiIdfor storing the Idwe had before and generated random Ids..... so now the Id on insert is generated by ODH However the insert does not work because in the datamodel the fields defined as required are:

ApiVersion,
OdhType,
ApiIdentifier

I saw the field ApiUrl on DataBrowser "Insert", this is an autogenerated field out of ApiVersion, ApiIdentifier and an optional ApiFilter

[gappc]

@RudiThoeni thank you for the changes, here is some feedback based on the TEST environment:

1. do we need the field ApiId at all, given that we moved to a generated ID field? If the field is not needed anymore, I would remove it (note that the ApiId seems to be a required field)
2. you mentioned that ApiUrl is generated. At the same time there is an array field PathParam that seems to have the same responsibility, at least it is not clear to me if both fields are needed and which one is used. What happens if the fields contradict each other?
3. how is ApiUrl autogenerated for paths that point to sub-resources (e.g. /MyNewResource/ID_XYZ/Address)? Do the sub-resource-paths then have to be part of the the ApiIdentifier (e.g. ApiIdentifier = MyNewResource/ID_XYZ/Address)?
4. what's the reason to have the OdhType field in the MetaData? If it is not strictly needed (at least it is not on the frontend), I would remove it

[RudiThoeni]

Hi

1) This field is not required, Have to check if it is present on the documentation when it is not there we can remove it........it was our old naming convention for the dataset name

2 + 3) Good point, in fact this information is doubled.... At the moment we have the ApiUrl generated out of ApiVersion/ApiIdentifier/(ApiFilter)

On Measuringpoints we have this example you mentioned on 3 it currently is handled this way "PathParam": [ "v1", "Weather", "Measuringpoint" ], "ApiVersion": "v1", "ApiIdentifier": "Weather/Measuringpoint", "ApiUrl": "https://api.tourism.testingmachine.eu/v1/Weather/Measuringpoint"

Now we have to decide which is the better option

• Using PathParam for the routes and removing Apiversion/ApiIdentifier. In this case we have an Array with all Routes inside, but we don't have a dedicated field like Apiversion. We only know that the first entry "should" be the ApiVersion.......

• Using ApiVersion, ApiIdentifier for the routes and removing PathParam We have a single field for each information, but as we have seen in the case above the ApiIdentifier could be "Weather/Measuringpoint" which is also not the best solution...

• Using both and transform PathParam into a readonly field........ I like to have the Array PathParam, what if i change it to an autogenerated field that is filled with ApiVersion, ApiIdentifier (and splits it if it has a /) ?

What is your opinion about it?

4) Maybe it could be a useful info to know what odhtype the endpoint is returning..... odhtype is always part of the _Meta Info of each record, and used in the Search api..... maybe let's keep it and remove just the Required attribute?

[gappc]

@RudiThoeni thank you for the answers, here are my ideas:

1. remove anything not necessary, it will simplify our lives down the road ;)

2+3. I'd opt to have PathParams only and no version information at all, because in reality the version information is just a part of the path and is used here as part of a convention - the version could be arbitrarily mapped (e.g. using HTTP headers). Using PathParams only gives us full flexibility and provides a simple mental model

4. at the moment I see no usage for the OdhType in the frontend and if it's not required sooner or later we have incomplete data. I'd opt to remove it - see also my answer for 1)

[RudiThoeni]

Hi

Ok all fine for me, reducing complexity is always welcome........ i will only check if the old id is not listed somewhere in de docs so we have decided to
Remove Fields

• ApiVersion
• ApiIdentifier
• ApiId
• OdhType
• The field ApiURL is autogenerated with the PathParam + ApiFilter
• set PathParm as Required

will write down here when done

[RudiThoeni]

I checked, and find nothing regarding to the old Ids, so i will remove the field ApiId However i will have to let OdhType there because at the moment my intern Record Count Update works with this field but i think this is not a problem, i set the field to not required .....

Maybe i can change also the field "ApiFilter": [ "source=noi" ], into a Dictionary like "ApiFilter": { "source": "noi" }, so it is clearer what do you think?

[RudiThoeni]

I deployed on Test

• removed ApiIdentifier
• removed ApiId
• removed ApiVersion
• set PathParam to required
• set OdhType to nullable
• changed Autogenerated ApiURL

I tried and It is now possible to insert data via databrowser (the passed ApiVersion is ignored, ApiUrl is autogenerated) also i saw the databrowser is missing a few fields (Sources, Output, Apiaccess)

let me know if this changes are ok

[gappc]

Hi @RudiThoeni, thank you for the changes, some of which came a bit as a surprise (see below) ;)

Here is my feedback:

• removed ApiIdentifier, ApiId, removed ApiVersion: removing ApiId and ApiVersion was fine. Maybe it was unclear what to do with ApiIdentifier, it should not have been removed because we relied on its value to find out if a dataset is a sub-dataset of another one. But don't worry, given that it was already removed, I've changed the implementation to adapt to that removal: now I check if a dataset has an ApiFilter set (means it is a sub-dataset) and if so, I try to find a dataset with same path params and no ApiFilter set (those i refer to as root-datasets). I think that implementation is better than the previous one because it eliminates possible user errors when setting the ApiIdentifier value
• set PathParam to required: nice
• OdhType: you mentioned that the count relies on this field being set. Does that mean, that the count is not correct if this field is not set? As a side node, I saw that the count for datasets and sub-datasets is the same (see e.g. Articles and Articles NOI here: https://databrowser.opendatahub.testingmachine.eu/dataset/table/tourism/v1/MetaData?rawfilter=and(in(PathParam.[*],%27Article%27))&rawsort=Shortname). This seems to be incorrect, because not all Articles are an Articles NOI and as a user I'd expect that the counts are different: count(Articles) >= count(Articles NOI). Is there some way we can approach and solve both problems, i.e. remove OdhType and have correct counts?
• changed Autogenerated ApiURL: nice
• SwaggerUrl: this field is still required (error if null: "The SwaggerUrl field is required."), I was not able to add a new record with this field being empty. Please make it optional

Other points I noted:

• Id: is defined as nullable in the OpenAPI defintion, which seems a bit odd given that it is the primary key. Maybe the reason is, that it should be empty for PUT requests? If that's the case, how do you handle an ID field that is set both in the body and in the URL in case of PUT?
• Shortname: I'd suggest to make Shortname required because it is used as title of the datasets in several places (except you had another intention for the field Shortname)

Before doing the next round of changes I think it would be a good idea if we had a meeting to discuss the open topics, what do you say?

[RudiThoeni]

Thank you for your feedback,

• Shortname set to required
• SwaggerUrl set to nullable true
• OdhType: we keep this field, the recordcount will be fixed on backend side don't worry about it
• Id as nullable, this was always the case in ODH..... why are you complaining about it now? since the Id is created on backend side, and on a POST the id is not passed, if set to not nullable the model check gives an error..... On a PUT the Id of the URL is taken. Using different models on POST and PUT seems overhead to me........ cheers

[sseppi]

Hi all, thank you all for the discussion and the changes you did.

I take the opportunity to notify that, in the next months we will integrate also the information about datasets of other domains in the Metadata API. This probably will lead to the need to extend and improve the metadata API accordingly to the needs that will emerge.

For this reason I suggest that in this iteration we do the changes we need to solve the bug/missing feature and then we create a dedicated issue/epic to discuss, plan and implement the future changes and optimization of the metadata API.

[gappc]

@RudiThoeni thank you for the changes, anything works. The nullable Id came to my mind as I was looking at the API spec, don't worry, no further requests from my side about that ;)

@sseppi agreed :+1:

[sseppi]

OK! Super. @gappc If I'm not wrong, we are ready to put this issue in ToDo lane.

[gappc]

@sseppi the "Add new" functionality of the Data Browser is tested and works for some endpoints (e.g. MetaData API), but it was not tested on all endpoints. I'm not sure if this issue refers to the MetaData endpoint only, or to all endpoints.

[sseppi]

@gappc this issue was referring only to the MetaData API, I will do a double-check test for the functionality on this endpoint and, if everything OK, close this issue.

Then I will organize a test af the "Add new" functionality on all other enpoint and, in case of need, open a dedicated issue for the other enpoints.

[sseppi]

@gappc I like it!

@pkritzinger and @Mazzolintis what do you think?

[pkritzinger]

@sseppi I honestly lost track regarding this issue. Can we have a look at it together tomorrow pls?

[sseppi]

OK, let see this issue all together tomorrow