-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add remaining acdd attrs issue 54 #78
base: main
Are you sure you want to change the base?
Conversation
datastore/protobuf/datastore.proto
Outdated
repeated Link links = 1; | ||
// --- END non-string metadata ----------------- | ||
|
||
// --- BEGIN string metadata (handleable with reflection) ----------------- | ||
string version = 2; | ||
string type = 3; | ||
string title = 4; | ||
string summary = 5; | ||
string keywords = 6; | ||
string keywords_vocabulary = 7 [json_name = "keywords_vocabulary"]; | ||
string license = 8; | ||
string conventions = 9; | ||
string naming_authority = 10 [json_name = "naming_authority"]; | ||
string creator_type = 101 [json_name = "creator_type"]; | ||
string creator_name = 12 [json_name = "creator_name"]; | ||
string creator_email = 13 [json_name = "creator_email"]; | ||
string creator_url = 14 [json_name = "creator_url"]; | ||
string institution = 15; | ||
string project = 16; | ||
string source = 17; | ||
string platform = 18; | ||
string platform_vocabulary = 19 [json_name = "platform_vocabulary"]; | ||
string standard_name = 20 [json_name = "standard_name"]; | ||
string unit = 21; | ||
string instrument = 22; | ||
string instrument_vocabulary = 23 [json_name = "instrument_vocabulary"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking schema change... which is probably fine, but we should let the ingestion team know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Will do!
datastore/protobuf/datastore.proto
Outdated
repeated string version = 3; | ||
repeated string type = 4; | ||
repeated string title = 5; | ||
repeated string summary = 6; | ||
repeated string keywords = 7; | ||
repeated string keywords_vocabulary = 8 [json_name = "keywords_vocabulary"]; | ||
repeated string license = 9; | ||
repeated string conventions = 10; | ||
repeated string naming_authority = 11 [json_name = "naming_authority"]; | ||
repeated string creator_type = 12 [json_name = "creator_type"]; | ||
repeated string creator_name = 13 [json_name = "creator_name"]; | ||
repeated string creator_email = 14 [json_name = "creator_email"]; | ||
repeated string creator_url = 15 [json_name = "creator_url"]; | ||
repeated string institution = 16; | ||
repeated string project = 17; | ||
repeated string source = 18; | ||
repeated string platform = 19; | ||
repeated string platform_vocabulary = 20 [json_name = "platform_vocabulary"]; | ||
repeated string standard_name = 21 [json_name = "standard_name"]; | ||
repeated string unit = 22; | ||
repeated string instrument = 23; | ||
repeated string instrument_vocabulary = 24 [json_name = "instrument_vocabulary"]; | ||
repeated string id = 25; | ||
repeated string data_id = 26 [json_name = "data_id"]; | ||
repeated string history = 27; | ||
repeated string metadata_id = 28 [json_name = "metadata_id"]; | ||
repeated string processing_level = 29 [json_name = "processing_level"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do something like:
map<string, repeated string> filters = 3;
Where the key is the metadata field you want to filter on, and the value is the list of values you want to match on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean avoiding explicit references in datastore.proto too? Interesting idea. Not sure if associative arrays (maps) are supported in proto files. If yes, I guess we would need to also change the database schema in the same fashion, no? So we would end up with two categories of attributes: 1) static/explicit attributes (like obs time, geo position, etc.), and 2) dynamic/implicit attributes. On the plus side it would be easier to alter the set of metadata at run time: no need for recompilation / redeployment - the datastore+api would simply adapt automatically to whatever the ingestor sends. On the minus side ... well I'm not sure. Maybe performance will suffer a little bit? And maybe it will be a bit trickier to support for non-trivial types (like the links attribute)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I guess I was too quick and misunderstood a bit. You mean to use a dynamic/implicit scheme only for the GetObsRequest
message! That makes good sense, and I'll look into ways to implement that (if necessary using a two-level array as a poor man's associative array etc.). Using dynamic/implicit naming for the whole shebang could also have its merits, but maybe try that at a later point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes indeed, I meant just for GetObsRequest
.
Although the idea of dynamic/implicit attributes in the DB is interesting as well. I guess you could say it means using a narrow table approach for the metadata as well? Anyway, probably good to leave this second part for later.
Attribute fields now use singular form (instruments -> instrument) for consistency and simplification (in particular when using reflection).
3c79df4
to
6675f69
Compare
The change in this branch most importantly adds search support (in
GetObservations
) for the rest of the currently supported metadata fields.The
TSMetadata.links
attribute is the only exception. The reason for this is that only basic string attributes are supported for now.By using reflection for the string attributes, we avoid explicit references to these attributes in all other files than
datastore.proto
andts-init.sql
. Thelinks
attribute thus still needs special handling in the Go code.NOTE: Attributes for temporal and spatial search (currently
obstime_instant
andgeo_point
) need special handling anyway. I.e. the reflection-based handling for the other attributes (which will eventually also cover thelinks
attribute) assumes a search filter in the form of a comma-separated list of patterns (case-insensitive and supporting basic wildcards (asterisks)).NOTE: The change in this branch organizes the two categories of metadata in all source code consistently by defining the non-string metadata first, followed by the string metadata (the latter handleable with reflection).