Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from
Mar 17, 2020
Merged

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

merged 2 commits into from
Mar 17, 2020

Conversation

jen-huang
Copy link
Contributor

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

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
@@ -183,7 +183,10 @@ export const StepConfigureDatasource: React.FunctionComponent<{
// Step B, configure inputs (and their streams)
// Assume packages only export one datasource for now
const ConfigureInputs =
packageInfo.datasources && packageInfo.datasources[0] ? (
packageInfo.datasources &&
Copy link
Member

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

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 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
Member

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

@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

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

@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