Skip to content

Comments

[Ingest] Add additional safeguards for data source wizard step 2#60426

Merged
jen-huang merged 2 commits intoelastic:masterfrom
jen-huang:ingest/input-check
Mar 17, 2020
Merged

[Ingest] Add additional safeguards for data source wizard step 2#60426
jen-huang merged 2 commits intoelastic:masterfrom
jen-huang:ingest/input-check

Conversation

@jen-huang
Copy link
Contributor

@jen-huang jen-huang commented Mar 17, 2020

Summary

This PR adds additional null checks for step 2 of the data source wizard. Issue surfaced from Endpoint package, which exports no inputs: elastic/package-registry#260 (comment)

image

@jen-huang jen-huang added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project Feature:Fleet Fleet team's agent central management project v7.7.0 labels Mar 17, 2020
@jen-huang jen-huang requested a review from a team March 17, 2020 18:05
@jen-huang jen-huang self-assigned this Mar 17, 2020
@elasticmachine
Copy link
Contributor

elasticmachine commented Mar 17, 2020

Pinging @elastic/ingest-management (Feature:Fleet)

@jen-huang jen-huang mentioned this pull request Mar 17, 2020
Merged
@jen-huang jen-huang requested review from a team and removed request for a team March 17, 2020 18:09
nchaulet
nchaulet reviewed Mar 17, 2020
View reviewed changes
...ns/ingest_manager/sections/agent_config/create_datasource_page/step_configure_datasource.tsx
// Assume packages only export one datasource for now
const ConfigureInputs =
packageInfo.datasources && packageInfo.datasources[0] ? (
packageInfo.datasources &&
Copy link
Member

@nchaulet nchaulet Mar 17, 2020

Choose a reason for hiding this comment

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

what about using typescript coalescing here(packageInfo.datasources?.[0]?.inputs?.length ?? false) ? (<>) :

jonathan-buttner
jonathan-buttner approved these changes Mar 17, 2020
View reviewed changes
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.

Looks good thank you!

image

@jen-huang
Copy link
Contributor Author

jen-huang commented Mar 17, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

kibanamachine commented Mar 17, 2020

💚 Build Succeeded

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

@jen-huang jen-huang merged commit f168b6a into elastic:master Mar 17, 2020
@jen-huang jen-huang deleted the ingest/input-check branch March 17, 2020 22:26
jen-huang added a commit to jen-huang/kibana that referenced this pull request Mar 17, 2020
@jen-huang @elasticmachine
Add additional safeguards for data source wizard step 2 (elastic#60426)
b864963 
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@jen-huang jen-huang mentioned this pull request Mar 17, 2020
Merged
@ruflin
Copy link
Contributor

ruflin commented Mar 18, 2020

I wonder if the user should ever get to this step 2? I think if someone wants to configure the endpoint package, we should send him directly to endpoint UI.

gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 18, 2020
@gmmorris
Merge branch 'master' into alerting/view-in-app
ff45d00 
* master:
  [ML] Re-enabling file upload telemetry (elastic#60418)
  [NP] Use local helper shortenDottedString for discover (elastic#60271)
  [Console] Fix for `_settings` and x-pack autocomplete (elastic#60246)
  Task/host enhancements (elastic#59671)
  [Search service] Asynchronous ES search strategy (elastic#53538)
  Index Action - Moved index params fields to connector config (elastic#60349)
  Edits UI text for ML nodes and job button (elastic#60184)
  Publish getIsNavDrawerLocked$ method on core chrome service. (elastic#60191)
  Disabled edit alert button on management ui for non registered UI alert types (elastic#60439)
  Revert "[Console] Fix bool filter autocompletions and refactor (elastic#60361)"
  [Console] Fix bool filter autocompletions and refactor (elastic#60361)
  Update ingest management team handle (elastic#60457)
  [IM] Use EuiCodeBlock to render index mapping (elastic#60420)
  Add additional safeguards for data source wizard step 2 (elastic#60426)
  [kbn/pm] don't fail when plugins are outside repo (elastic#60164)
  upgrade react-use (elastic#60427)
  Remove link to old settings (elastic#60326)
  Update app arch CODEOWNERS items. (elastic#60396)
  [ML] Fixing custom urls to dashboards (elastic#60355)
  Update the ems-client dependency to 7.7.0 (elastic#59936)
@paul-tavares
Copy link
Contributor

paul-tavares commented Mar 18, 2020

@ruflin I was wondering the same thing. Or, only allow the user to add a name + description on the ingest side and show a message that Endpoint datasource can be configured on the endpoint app after being added to the Agent Config.

jen-huang added a commit that referenced this pull request Mar 18, 2020
@jen-huang @elasticmachine
Add additional safeguards for data source wizard step 2 (#60426) (#60452
a0840cf 
)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@jen-huang
Copy link
Contributor Author

jen-huang commented Mar 18, 2020

Re: Step 2 when using Endpoint - we discussed this during design sync today, final flow TBD but I think we will still keep this overall flow with a message saying that the data source will be created with a recommended Endpoint policy.

@paul-tavares
Copy link
Contributor

paul-tavares commented Mar 19, 2020

@jen-huang sounds good :)
My next effort will likely be around writing the endpoint policy data (json) to the datasource and may need some guidance as to where in that data structure it should be saved to

@jen-huang jen-huang added the Team:Fleet Team label for Observability Data Collection Fleet team label Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nchaulet nchaulet nchaulet approved these changes

@jonathan-buttner jonathan-buttner jonathan-buttner approved these changes

@paul-tavares paul-tavares paul-tavares approved these changes

Assignees

@jen-huang jen-huang

Labels

Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project Feature:Fleet Fleet team's agent central management project release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.7.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@jen-huang @elasticmachine @kibanamachine @ruflin @paul-tavares @nchaulet @jonathan-buttner