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 content pack import/export for lookup tables, caches and adapters #3892

Merged
merged 4 commits into from Jun 13, 2017

Conversation

Projects
3 participants
@bernd
Member

bernd commented Jun 6, 2017

This adds content pack import/export for lookup tables, caches and data adapters.

Closes #3833

@bernd bernd added this to the 2.3.0 milestone Jun 6, 2017

@bernd bernd requested a review from kroepke Jun 6, 2017

@bernd bernd added the in progress label Jun 6, 2017

@bernd bernd changed the title from [WIP] Add content pack import/export for lookup tables, caches and adapters to Add content pack import/export for lookup tables, caches and adapters Jun 7, 2017

@bernd bernd added ready-for-review and removed in progress labels Jun 7, 2017

@bernd bernd added this to Done in Lookup Tables Jun 7, 2017

@bernd bernd moved this from Done to In Progress in Lookup Tables Jun 7, 2017

@kroepke

This comment has been minimized.

Member

kroepke commented Jun 9, 2017

needs change for moved/renamed classes in #3873

bernd added some commits Jun 6, 2017

Add "content_pack" field to lookup table/cache/adapter DTOs
- Set the "content_pack" field when importing from a content pack
- Show a content pack marker on the table/cache/adapter overview
  and detail views

@bernd bernd force-pushed the lut-contentpacks branch from b1d9c87 to b056171 Jun 12, 2017

@bernd

This comment has been minimized.

Member

bernd commented Jun 12, 2017

@kroepke I adjusted the PR for the moved/renamed classes in #3873

},
render() {
const style = { marginLeft: this.props.marginLeft, marginRight: this.props.marginRight };

This comment has been minimized.

@kroepke

kroepke Jun 13, 2017

Member

@edmundoa What do you think, would it be worthwhile to give ContentPackMarker either two components as props or two children to get rid of the inline styles?
Or is that overkill?

Also see comment below for an example.

<h2>{cache.title} <small>({plugin.displayName})</small></h2>
<h2>
{cache.title}
<ContentPackMarker contentPack={cache.content_pack} marginLeft={5} />

This comment has been minimized.

@kroepke

kroepke Jun 13, 2017

Member

Related to above: could we give it a left and right sibling prop instead of padding with spaces and inline styles?

This comment has been minimized.

@edmundoa

edmundoa Jun 13, 2017

Member

In that case we would need a different component name, maybe something like EntityTitle or something like that, where the ContentPackMarker is just another thing to be included. Otherwise it would feel odd to me, having a ContentPackMarker component which takes the title and other information.

This comment has been minimized.

@kroepke

kroepke Jun 13, 2017

Member

After some discussion we've decided that we can postpone the creation of additional components for now.
It's really outside the scope of this feature and ideally we would restructure the content packs anyway to make them extensible.

@kroepke kroepke merged commit b974b72 into master Jun 13, 2017

4 checks passed

ci-web-linter Jenkins build graylog-pr-linter-check 1733 has succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
graylog-project/pr Jenkins build graylog-project-pr-snapshot 213 has succeeded
Details

@kroepke kroepke deleted the lut-contentpacks branch Jun 13, 2017

@bernd bernd moved this from In Progress to Done in Lookup Tables Jun 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment