Add dataset_types to package info #511

ruflin · 2020-06-15T13:48:04Z

To be updated

elasticmachine · 2020-06-15T13:51:22Z

💚 Build Succeeded

Expand to view the summary

Build stats

Build Cause: [Pull request #511 updated]
Start Time: 2020-06-22T08:22:06.265+0000
Duration: 11 min 39 sec

Test stats 🧪

Test	Results
Failed	0
Passed	109
Skipped	0
Total	109

ruflin · 2020-06-16T11:06:23Z

As this does not remove / change the /categories endpoint yet, this can be merged and the change in Kibana can follow. Issue filed here: elastic/kibana#69252

docs/api/package.json

docs/api/search-datasetType-logs.json

main.go

mtojek · 2020-06-16T11:46:31Z

search.go

@@ -48,6 +49,12 @@ func searchHandler(packagesBasePath string, cacheTime time.Duration) func(w http
 				}
 			}

+			if v := query.Get("dataset.type"); v != "" {


nit: dataset_type ?

This is by design. I want to stay as close as possible to the actual fields used in the events. So the query is really dataset.type=logs like it is inside Elasticsearch.

mtojek · 2020-06-16T12:00:38Z

types.go

+
+		data, err := getTypesOutput(types)
+		if err != nil {
+			notFoundError(w, err)


I think you can get here a marshaling error. It would be great if the app logs this one.

mtojek · 2020-06-16T12:01:44Z

util/package.go

@@ -50,6 +50,8 @@ type Package struct {
 	Datasources   []Datasource `config:"datasources,omitempty" json:"datasources,omitempty" yaml:"datasources,omitempty"`
 	Download      string       `json:"download" yaml:"download,omitempty"`
 	Path          string       `json:"path" yaml:"path,omitempty"`
+	// This is in purpose `dataset.types` with an "s" as it is a summary of the existing dataset types


nit: on purpose:

mtojek · 2020-06-16T12:02:27Z

util/package.go

@@ -50,6 +50,8 @@ type Package struct {
 	Datasources   []Datasource `config:"datasources,omitempty" json:"datasources,omitempty" yaml:"datasources,omitempty"`
 	Download      string       `json:"download" yaml:"download,omitempty"`
 	Path          string       `json:"path" yaml:"path,omitempty"`
+	// This is in purpose `dataset.types` with an "s" as it is a summary of the existing dataset types
+	DatasetTypes []string `json:"dataset.types" yaml:"dataset.types,omitempty"`


should both ones have the omitempty annotation?

yes, and I realised that we don't need the yaml part as it never shows up in yaml.

util/package.go

The type API endpoint exposes the number of packages with a dataset of a certain type exists. Today this is what is exposed by the category endpoint but this will be repurposed. The value of the type is based on the dataset.type field, indirectly the type definition inside the dataset manifest. It is expected that Kibana switches over to using this endpoint and then the category endpoint can be refactored. What is currently exposed under category is not really categories.

ruflin

I removed the /types endpoint from this PR as it turned out we don't need it for the next release. But we potentially need the types info of the dataset types in a package on the search endpoint. Will update the PR description and add more changes for it.

docs/api/package.json

docs/api/search-datasetType-logs.json

search.go

ruflin · 2020-06-22T06:30:15Z

util/package.go

@@ -50,6 +50,8 @@ type Package struct {
 	Datasources   []Datasource `config:"datasources,omitempty" json:"datasources,omitempty" yaml:"datasources,omitempty"`
 	Download      string       `json:"download" yaml:"download,omitempty"`
 	Path          string       `json:"path" yaml:"path,omitempty"`
+	// This is in purpose `dataset.types` with an "s" as it is a summary of the existing dataset types


ruflin · 2020-06-22T06:31:05Z

util/package.go

@@ -50,6 +50,8 @@ type Package struct {
 	Datasources   []Datasource `config:"datasources,omitempty" json:"datasources,omitempty" yaml:"datasources,omitempty"`
 	Download      string       `json:"download" yaml:"download,omitempty"`
 	Path          string       `json:"path" yaml:"path,omitempty"`
+	// This is in purpose `dataset.types` with an "s" as it is a summary of the existing dataset types
+	DatasetTypes []string `json:"dataset.types" yaml:"dataset.types,omitempty"`


yes, and I realised that we don't need the yaml part as it never shows up in yaml.

util/package.go

ruflin · 2020-06-22T07:02:34Z

Waiting for #529 to get in to continue work on this PR.

I converted above dataset.types to dataset: { types but I'm torn if this is the right call. The reason I converted it to an object is to align with dataset.* object but at the same time inside this object there is not types (plural) and it kind of implies a dataset could have more than one type (it can't). So I'm getting back to the proposal from @mtojek to use dataset_types. Will adjust it.

ruflin · 2020-07-02T13:24:44Z

I'm closing this for now as it turned out we will not need it yet for the next release.

ruflin self-assigned this Jun 15, 2020

ruflin force-pushed the types-endpoint branch from 74707b0 to 79cba8c Compare June 16, 2020 11:02

ruflin requested review from mtojek and jen-huang June 16, 2020 11:03

ruflin mentioned this pull request Jun 16, 2020

[Ingest Manager] Use /types instead of /categories registry endpoint elastic/kibana#69252

Closed

mtojek reviewed Jun 16, 2020

View reviewed changes

ruflin added 2 commits June 22, 2020 08:34

remove types endpoint

74ffc0f

ruflin force-pushed the types-endpoint branch from e3d598e to 74ffc0f Compare June 22, 2020 06:35

ruflin commented Jun 22, 2020

View reviewed changes

adjust to dataset_types

bb6374a

ruflin changed the title ~~Add /types API endpoint~~ Add dataset_types to package info Jun 22, 2020

fix tests

1ea5f1d

ruflin mentioned this pull request Jun 30, 2020

[Meta] Planned changes to the manifest format #517

Closed

29 tasks

ruflin closed this Jul 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dataset_types to package info #511

Add dataset_types to package info #511

ruflin commented Jun 15, 2020 •

edited

Loading

elasticmachine commented Jun 15, 2020 •

edited

Loading

Build stats

Test stats 🧪

ruflin commented Jun 16, 2020

mtojek Jun 16, 2020

ruflin Jun 16, 2020

mtojek Jun 16, 2020

mtojek Jun 16, 2020

ruflin Jun 22, 2020

mtojek Jun 16, 2020

ruflin Jun 22, 2020

ruflin left a comment

ruflin Jun 22, 2020

ruflin Jun 22, 2020

ruflin commented Jun 22, 2020

ruflin commented Jul 2, 2020

Add dataset_types to package info #511

Add dataset_types to package info #511

Conversation

ruflin commented Jun 15, 2020 • edited Loading

elasticmachine commented Jun 15, 2020 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

ruflin commented Jun 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Jun 22, 2020

ruflin commented Jul 2, 2020

ruflin commented Jun 15, 2020 •

edited

Loading

elasticmachine commented Jun 15, 2020 •

edited

Loading