-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 - CSV #7372
Add Data - CSV #7372
Conversation
…r to processor and pipeline
Updates the ingest POST endpoint to use Kibana processor schema
Ingest pipeline setup client
…e filebeat installation instructions
Implements a pattern checker directive for use in the Add Data UI
Fixing a few extra require statements hanging about in the ingest code
[es6ify] re-apply automated transforms after ingest work
jenkins, test it |
[add data] fixes grok processor converter
jenkins, test it |
@tylersmalley alright, no conflicts or test failures! Could you take another look? |
@BigFunger I don't know how I could have left you out of this one :) |
@Bargs I can only assume it was through an act of kindness. |
I uploaded over a dozen random CSV's from data.gov and didn't run into any issues. Obviously, the flow will be much improved when the ingest pipeline is merged in, but the messaging is good and allows you to manually correct issues and re-upload. Functionally, this LGTM. |
Resolved conflicts, still need to move around some add data files.
@tylersmalley @BigFunger I updated this PR to add the Upload CSV wizard to the new management screen. Could you guys take one more look? Assuming there are no issues this should be the last change required. One note: some of the colors are a little off in the context of the new management screen, but I think it makes sense to handle this along with the clean up of management as a whole, rather than adding a bunch of super specific overrides to Add Data right now. |
.nav-buttons { | ||
float:right; | ||
} | ||
.nav-buttons { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure you want to define this style here? Before it was confined to the kbn-management-indices
directive. Now it's applied to the entire application. Also, where is this class/style being used?
Functionally LGTM, just had a small less/css question. |
@@ -1,4 +1,4 @@ | |||
import PluginsKibanaSettingsSectionsIndicesRefreshKibanaIndexProvider from 'plugins/kibana/settings/sections/indices/_refresh_kibana_index'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
* Switched data/upload url to data/csv for better future proofing * Removed old unnecessary css rule * Fixed issue with management tab link removing the wizard's app state
@tylersmalley @BigFunger I addressed all of your feedback, please take another look. |
As soon as this turns green, it LGTM! 🐑 🇮🇹 |
jenkins, test it |
LGTM! |
Add Data - CSV Former-commit-id: 15a4fa1
Fixes #6541
This PR merges the long running feature/ingest branch into master. It contains code for both the Tail A File and CSV Add Data wizards, but only the CSV wizard is currently enabled. The CSV wizard is a simplified version of what's described in the ticket. It omits the pipeline building step because we want to implement editable pipelines before making that feature available.
This looks massive, but all of the code has been reviewed in separate PRs that targeted the feature/ingest branch. This PR mostly just needs eyes on the UI itself to ensure it's ready to merge into master.