Skip to content

Conversation

@kevinlog
Copy link
Contributor

@kevinlog kevinlog commented Dec 11, 2020

Summary

This PR adds the hidden field to package installation which can be used to indicate that a data_stream should be hidden.

Example package PR that will use this field: elastic/endpoint-package#118

See screenshot after installing a package with hidden set to true
image

Checklist

Delete any items that are not applicable to this PR.

@kevinlog kevinlog requested a review from a team December 11, 2020 18:04
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Dec 11, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@kevinlog kevinlog added release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Dec 11, 2020
@kevinlog kevinlog changed the title Installation hidden field [Fleet] Installation hidden field Dec 11, 2020
@kevinlog kevinlog changed the title [Fleet] Installation hidden field [Fleet] Installation of hidden field Dec 11, 2020
@ruflin
Copy link
Contributor

ruflin commented Dec 14, 2020

This PR does not prefix hidden data streams with a . yet it seems. @tsg Should we do it directly?

@ruflin
Copy link
Contributor

ruflin commented Dec 14, 2020

@kevinlog I got confused at first by the title of the PR as it is about hidden data streams and not fields. I understand that it is a field in the manifest ;-)

@ruflin ruflin requested a review from skh December 14, 2020 08:21
@kevinlog
Copy link
Contributor Author

@elasticmachine merge upstream

pipelineName?: string | undefined;
packageName: string;
composedOfTemplates: string[];
hidden?: boolean | false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't boolean | false make the field non-optional for TS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the field can still be optionally passed - take a look at this unit test case:
https://github.com/elastic/kibana/pull/85703/files#diff-f1bfa3ccb84f6e245a1f2a762eceeb3f2372bd454578568ce84a43c9c2c47fffR85

I'm thinking that we can drop the | false part or update it to | undefined

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer dropping the | false then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in both places

Copy link
Contributor

@skh skh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one question about types. LGTM 👍

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Distributable file count

id before after diff
default 47129 47889 +760

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@jonathan-buttner jonathan-buttner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just a note, once you merge one of these you might cause conflicts in your other PR because I think we're touching a couple of the same functions. So merge them slowly and see if conflicts need to be resolved, otherwise we might break master.

@kevinlog kevinlog merged commit 06993c4 into elastic:master Dec 14, 2020
@kevinlog kevinlog deleted the installation-hidden-field branch December 14, 2020 18:46
kevinlog added a commit that referenced this pull request Dec 14, 2020
Co-authored-by: nnamdifrankie <franklin.ejoh@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: nnamdifrankie <franklin.ejoh@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.11.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants