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.

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

[add data] fixed pipeline output when no processors in error #6817

Merged
merged 3 commits into from
Apr 7, 2016

Conversation

BigFunger
Copy link
Contributor

Summary

Before this PR, if the pipeline contained no processors or if any of the processors had an error, then the pipeline output at the top of the page would be empty. This PR fixes that.

Changes

  • Added a hasValidOutput getter to the base processor class. This returns false if the processor has an error or is locked.
  • Modified the processor.updateOutput function to grab the outputObject from the last processor that hasValidOutput is true.
  • Modified the processor.updateOutput function to use the pipeline.input object if there are no processors in the pipeline with hasValidOutput.
  • Modified the processor.updateOutput function
    • If there are no processors, it uses the input
    • If there is no error, it uses the last processors output
    • If there is an error, it uses the output from the processor before the error (if one exists)
  • Updated the processor test suite
    • Uses the Set processor class instead of a dummy class.
    • Added additional test cases for updateOutput

@BigFunger BigFunger added review Feature:Add Data Add Data and sample data feature on Home labels Apr 7, 2016
const pipeline = new Pipeline();

pipeline.updateOutput();
expect(pipeline.output).to.be(undefined);
expect(pipeline.output).to.be(pipeline.input);
Copy link
Contributor

Choose a reason for hiding this comment

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

To make this test more robust I think you should explicitly set pipeline.input before calling updateOutput(). As it stands, this test would pass if both input and output were just initialized to undefined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

test case updated.

@Bargs Bargs assigned BigFunger and unassigned Bargs Apr 7, 2016
@BigFunger BigFunger assigned Bargs and unassigned BigFunger Apr 7, 2016
@BigFunger BigFunger force-pushed the ingest-pipeline-output branch from 271661a to be93f5c Compare April 7, 2016 16:32
@Bargs
Copy link
Contributor

Bargs commented Apr 7, 2016

Sorry I didn't catch this before...

It looks like a compile error causes the output to revert to its initial state because all processors are locked or in error state when there's a compile error. I think updateOutput needs to find the first processor that's in an error state and then grab the output from the processor before it (if one exists, otherwise pipeline.input), regardless of whether it's locked or not.

@BigFunger
Copy link
Contributor Author

Updated first comment to reflect the changes made.

@Bargs
Copy link
Contributor

Bargs commented Apr 7, 2016

LGTM! 🎉

@Bargs Bargs merged commit 00a5bb2 into elastic:feature/ingest Apr 7, 2016
cee-chen added a commit that referenced this pull request May 31, 2023
This is a backport EUI upgrade to Kibana v8.8.1 containing an
EuiDataGrid bugfix requested by the Discover team:
elastic/eui#6804 (comment)

## [`77.1.4`](https://github.com/elastic/eui/tree/v77.1.4)

- Updated `EuiDataGrid` to only render screen reader text announcing
cell position if the cell is currently focused. This should improve the
ability to copy and paste multiple cells without SR text.
([#6817](elastic/eui#6817))
cee-chen added a commit that referenced this pull request Jun 5, 2023
## Summary

`eui@81.0.0` ⏩ `eui@81.2.0`

- Most changes to source code in this PR are CSS cleanups/deprecations
in `EuiSuperDatePicker`/`EuiDatePickerRange`
- One team (ML) had a `inline` specific usage of `EuiDatePickerRange`
that they reached out to us about via Slack, and that we have fixed in
this PR.
- All other usages of date picker components should have remained
working as-is with no changes, but please ping us if you see otherwise!

___

## [`81.2.0`](https://github.com/elastic/eui/tree/v81.2.0)

- Updated `EuiSuperDatePicker` to accept an object configuration for
`isDisabled` ([#6821](elastic/eui#6821))

**Bug fixes**

- Fixed broken `EuiSuperDatePicker` styles
([#6821](elastic/eui#6821))

## [`81.1.0`](https://github.com/elastic/eui/tree/v81.1.0)

- Added `EuiInlineEditText` and `EuiInlineEditTitle` components
([#6757](elastic/eui#6757))
- Updated `EuiDatePickerRange` to support `inline` display
([#6795](elastic/eui#6795))
- Added an `onError` callback prop to `EuiErrorBoundary`
([#6810](elastic/eui#6810))
- Updated `EuiDataGrid` to only render screen reader text announcing
cell position if the cell is currently focused. This should improve the
ability to copy and paste multiple cells without SR text.
([#6817](elastic/eui#6817))

**Bug fixes**

- Fixed `EuiDatePicker`'s `inline` display to correctly render and
prevent user interaction when `disabled` or `readOnly`
([#6795](elastic/eui#6795))
- Fixed `EuiDatePicker`'s `inline` display to correctly render
`isInvalid` and `isLoading` icons
([#6795](elastic/eui#6795))

**CSS-in-JS conversions**

- Converted `EuiDatePickerRange` to Emotion
([#6795](elastic/eui#6795))

---------

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
Feature:Add Data Add Data and sample data feature on Home review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants