Add experimental releases for packages and datasets #382

ruflin · 2020-04-24T18:43:01Z

So far the available release types were beta and ga. In this PR experimental is added to the list and set as default.

Having experimental around for packages provides some interesting options. Experimental packages are not shown in the search endpoint by default. To query for the experimental package the flag experimental=true has to be added. This allows to publish some packages without directly making it available to everyone.

The idea is that we introduce in EPM a setting "Experimental" that users can enable to also see and install experimental packages.

On the dataset level also experimental datasets exist. My expectation here is that these would not be shown by the Config Builder as long as Experimental:true is not enabled.

So far the available release types were beta and ga. In this PR experimental is added to the list and set as default. Having experimental around for packages provides some interesting options. Experimental packages are not shown in the search endpoint by default. To query for the experimental package the flag `experimental=true` has to be added. This allows to publish some packages without directly making it available to everyone. The idea is that we introduce in EPM a setting "Experimental" that users can enable to also see and install experimental packages. On the dataset level also experimental datasets exist. My expectation here is that these would not be shown by the Config Builder as long as Experimental:true is not enabled.

ruflin · 2020-04-24T18:44:07Z

@mtojek If we go forward with this, my thinking would be that soon we switch the generator to have experimental as default, but first we need the setting in Kibana
@neptunian @nchaulet Would be such a setting for EPM possible?
@jen-huang WDYT about the dataset part that is not shown if experimental?

nchaulet · 2020-04-24T18:59:59Z

For EPM I think we can pass the flag if the user provide it in the query string for example.

On the dataset level also experimental datasets exist

Do you mean we can have experimental dataset inside a package that is not experimental?

Also regarding name what do you think of using alpha instead of experimental I think it sounds more a versioning concern.

mtojek · 2020-04-27T07:40:06Z

@mtojek If we go forward with this, my thinking would be that soon we switch the generator to have experimental as default, but first we need the setting in Kibana

import-beats bases on the release tag in Beats. We can ignore the Beats tag at all and set everywhere "experimental" for every package.

mtojek · 2020-04-27T07:40:51Z

CHANGELOG.md

@@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Add `removable` flag to package manifest. Default is true. [#359](https://github.com/elastic/integrations-registry/pull/359)
 * Add stream template to package json. [#335](https://github.com/elastic/integrations-registry/pull/335)
 * Add support for multiple inputs per dataset. [#346](https://github.com/elastic/integrations-registry/pull/346)
+* Add experimental releases for packages and datasets. [#](https://github.com/elastic/integrations-registry/pull/)


nit: missing PR reference

mtojek · 2020-04-27T07:42:17Z

categories.go

+		query := r.URL.Query()
+		var experimental bool
+		// Read query filter params which can affect the output
+		if len(query) > 0 {


I think you can extract this block to a separate method.

you mean to reuse it for categories and search?

Yes and try to adjust the production code to be as concise as possible (SRP, KISS).

The query part between search and categories is not identical, the part that could be reused is the check around the flag. I can extract that part if you prefer.

Side note: I don't consider copying code as a fundamentally bad thing ;-)

Don't need to spend time right now, we can refactor it later on.

yes, we have to.

mtojek · 2020-04-27T07:44:31Z

search.go

+			if v := query.Get("experimental"); v != "" {
+				if v != "" {
+					// In case of error, keep it false
+					experimental, _ = strconv.ParseBool(v)


I'm not fan of swallowing errors... this should reflect in HTTP 400 Bad Request.

You are right, if wrong params are submitted, we should just error and not assume defaults like we do here. As this applies to also the other api params, ok if I follow up with it?

Sounds good!

ruflin

@nchaulet At Elastic we often use the 3 stages of a product: experimental, beta, ga. I tried to align it to this.

ruflin · 2020-04-27T10:22:43Z

categories.go

+		query := r.URL.Query()
+		var experimental bool
+		// Read query filter params which can affect the output
+		if len(query) > 0 {


you mean to reuse it for categories and search?

ruflin

@mtojek Thanks for the 👍 Will merge as soon as green (pushed the changelog) but then we have to follow up with the error fixes and simplifying the code.

ruflin · 2020-04-27T11:46:04Z

categories.go

+		query := r.URL.Query()
+		var experimental bool
+		// Read query filter params which can affect the output
+		if len(query) > 0 {


yes, we have to.

ruflin · 2020-04-30T07:09:44Z

I filed elastic/kibana#64869 as a follow up for Kibana.

jfsiii · 2020-04-30T12:24:59Z

I missed this initially, but this reads like there would be 3 or N flags (experimental, beta, ga). Is that correct?

Can multiple values be supplied? e.g. ?experimental=true&ga=true&beta=true

Since these all correspond to a single release field, I'd expect a single param, like release=experimental. Multiple could be ? release=experimental,ga,beta or ?release=experimental&release=ga&release=beta

ruflin · 2020-04-30T12:37:27Z

Currently it is 3, I could see a 4th one coming which is deprecated.

Lets assume we go with the release param. If no query is provided, it would return beta,ga by default. Now what happens if release=experimental is used, will it only return experimental ones or also beta,ga?

I'm now wondering if both flags can be come useful. release as you suggest it is a filter. experimental=true as a boolean is a shortcut for release=experimental,beta,ga.

ruflin requested review from nchaulet, neptunian and mtojek April 24, 2020 18:43

ruflin self-assigned this Apr 24, 2020

mtojek reviewed Apr 27, 2020

View reviewed changes

ruflin commented Apr 27, 2020

View reviewed changes

mtojek self-requested a review April 27, 2020 11:43

mtojek approved these changes Apr 27, 2020

View reviewed changes

update changelog

69e5a7e

ruflin commented Apr 27, 2020

View reviewed changes

ruflin merged commit fb405e7 into elastic:master Apr 27, 2020

ruflin deleted the experimental-flag branch April 27, 2020 11:52

ruflin mentioned this pull request Apr 30, 2020

[Fleet] Have an experimental config option for packages elastic/kibana#64869

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add experimental releases for packages and datasets #382

Add experimental releases for packages and datasets #382

ruflin commented Apr 24, 2020

ruflin commented Apr 24, 2020

nchaulet commented Apr 24, 2020

mtojek commented Apr 27, 2020

mtojek Apr 27, 2020

mtojek Apr 27, 2020

ruflin Apr 27, 2020

mtojek Apr 27, 2020

ruflin Apr 27, 2020

mtojek Apr 27, 2020

ruflin Apr 27, 2020

mtojek Apr 27, 2020

ruflin Apr 27, 2020

mtojek Apr 27, 2020

ruflin left a comment

ruflin Apr 27, 2020

ruflin left a comment

ruflin Apr 27, 2020

ruflin commented Apr 30, 2020

jfsiii commented Apr 30, 2020

ruflin commented Apr 30, 2020

Add experimental releases for packages and datasets #382

Add experimental releases for packages and datasets #382

Conversation

ruflin commented Apr 24, 2020

ruflin commented Apr 24, 2020

nchaulet commented Apr 24, 2020

mtojek commented Apr 27, 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

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

ruflin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Apr 30, 2020

jfsiii commented Apr 30, 2020

ruflin commented Apr 30, 2020