You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by ottobackwards <gi...@git.apache.org> on 2017/09/03 03:28:42 UTC
[GitHub] metron pull request #731: METRON-1146 Refactor for ParserExtensionConfig and...
GitHub user ottobackwards opened a pull request:
https://github.com/apache/metron/pull/731
METRON-1146 Refactor for ParserExtensionConfig and Rest get all call
This a PR for some cleanup on the 1136 Feature Branch
- return a list from GET ( getAllParsers ) and not a map.
- fix names in Parser Extension Configs to be consistant with extension/extensions
- add a field just called identifier, to be used instead of assembly name
- differentiate identifier/assemblyname
##
- [x] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
- [x] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
- [x] Have you ensured that the full suite of tests and checks have been executed in the root metron
- [x] Have you written or updated unit tests and or integration tests to verify your changes?
- [x] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ottobackwards/metron ext-refactor-names-getall
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/731.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #731
----
commit 0d3b4c01fa41a3ee888a4debf249e62f82f2a184
Author: Otto Fowler <ot...@gmail.com>
Date: 2017-09-03T03:17:46Z
Refactor:
- return a list from GET ( getAllParsers ) and not a map.
- fix names in Parser Extension Configs to be consistant with extension/extensions
- add a field just called identifier, to be used instead of assembly name
- differentiate identifier/assemblyname
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #731: METRON-1136 Refactor for ParserExtensionConfig and...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards closed the pull request at:
https://github.com/apache/metron/pull/731
---
[GitHub] metron issue #731: METRON-1146 Refactor for ParserExtensionConfig and Rest g...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron/pull/731
+1 Thanks, Otto
---
[GitHub] metron pull request #731: METRON-1146 Refactor for ParserExtensionConfig and...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/metron/pull/731#discussion_r137316957
--- Diff: metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/ExtensionServiceImpl.java ---
@@ -528,20 +527,26 @@ private void rollBackPatternsFromHdfs(InstallContext context) throws Exception{
private void writeExtensionConfiguration(InstallContext context) throws Exception {
ParserExtensionConfig config = new ParserExtensionConfig();
config.setParserExtensionParserName(context.extensionParserNames);
- config.setExtensionsBundleID(context.bundleID.get());
- config.setExtensionsBundleVersion(context.bundleVersion.get());
+ config.setExtensionIdentifier(context.extensionIdentifier.get());
+ config.setExtensionBundleID(context.bundleID.get());
+ config.setExtensionBundleVersion(context.bundleVersion.get());
config.setExtensionBundleName(context.bundleName.get());
config.setExtensionAssemblyName(context.extensionPackageName.get());
config.setDefaultParserConfigs(context.defaultParserConfigs.get());
- config.setDefaultEnrichementConfigs(context.defaultEnrichmentConfigs.get());
+ config.setDefaultEnrichmentConfigs(context.defaultEnrichmentConfigs.get());
config.setDefaultIndexingConfigs(context.defaultIndexingConfigs.get());
if(context.defaultElasticSearchTemplates.isPresent()) {
config.setDefaultElasticSearchTemplates(context.defaultElasticSearchTemplates.get());
}
- ConfigurationsUtils.writeParserExtensionConfigToZookeeper(context.extensionPackageName.get(),config.toJSON().getBytes(), client);
+ ConfigurationsUtils.writeParserExtensionConfigToZookeeper(context.extensionIdentifier.get(),config.toJSON().getBytes(), client);
--- End diff --
While working with the config, the fact that I had picked a field to use as the identifier, but was also something else seemed confusing. Making the identifier explicit, and also making that field what it should be ( the assemblyName/packageName ) without truncating it seems to me to make it more understandable.
---
[GitHub] metron pull request #731: METRON-1146 Refactor for ParserExtensionConfig and...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron/pull/731#discussion_r137320162
--- Diff: metron-interface/metron-rest/src/main/java/org/apache/metron/rest/controller/ParserExtensionController.java ---
@@ -90,8 +91,8 @@
@ApiOperation(value = "Retrieves all ParserExtensionConfigs from Zookeeper")
@ApiResponse(message = "Returns all ParserExtensionConfigs", code = 200)
@RequestMapping(method = RequestMethod.GET)
- ResponseEntity<Map<String, ParserExtensionConfig>> findAll() throws RestException {
- return new ResponseEntity<Map<String, ParserExtensionConfig>>(extensionService.getAllParserExtensions(), HttpStatus.OK);
+ ResponseEntity<List<ParserExtensionConfig>> findAll() throws RestException {
--- End diff --
Makes sense. Its cleaner this way. (Even if the real reason is that you aren't good enough with Angular. Ha)
---
[GitHub] metron issue #731: METRON-1136 Refactor for ParserExtensionConfig and Rest g...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/731
I just got done doing that! Luckily the scripts that I use have you verify everything, and I caught the different message :)
---
[GitHub] metron pull request #731: METRON-1146 Refactor for ParserExtensionConfig and...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/metron/pull/731#discussion_r137316439
--- Diff: metron-interface/metron-rest/src/main/java/org/apache/metron/rest/controller/ParserExtensionController.java ---
@@ -90,8 +91,8 @@
@ApiOperation(value = "Retrieves all ParserExtensionConfigs from Zookeeper")
@ApiResponse(message = "Returns all ParserExtensionConfigs", code = 200)
@RequestMapping(method = RequestMethod.GET)
- ResponseEntity<Map<String, ParserExtensionConfig>> findAll() throws RestException {
- return new ResponseEntity<Map<String, ParserExtensionConfig>>(extensionService.getAllParserExtensions(), HttpStatus.OK);
+ ResponseEntity<List<ParserExtensionConfig>> findAll() throws RestException {
--- End diff --
I have another branch where I am doing work on extending the management ui to manage extensions. I had a lot of trouble consuming the maps. The map is the configurations with a field pulled out as the key, so making it a list made it work, similar to the getAll for parser configurations, and really didn't lose anything.
TL;DR
I am not good enough at angular to consume the maps correctly so I refactored.
---
[GitHub] metron pull request #731: METRON-1146 Refactor for ParserExtensionConfig and...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron/pull/731#discussion_r137308842
--- Diff: metron-interface/metron-rest/src/main/java/org/apache/metron/rest/controller/ParserExtensionController.java ---
@@ -90,8 +91,8 @@
@ApiOperation(value = "Retrieves all ParserExtensionConfigs from Zookeeper")
@ApiResponse(message = "Returns all ParserExtensionConfigs", code = 200)
@RequestMapping(method = RequestMethod.GET)
- ResponseEntity<Map<String, ParserExtensionConfig>> findAll() throws RestException {
- return new ResponseEntity<Map<String, ParserExtensionConfig>>(extensionService.getAllParserExtensions(), HttpStatus.OK);
+ ResponseEntity<List<ParserExtensionConfig>> findAll() throws RestException {
--- End diff --
What is the logic for changing this from a map to a list?
---
Re: [GitHub] metron pull request #731: METRON-1146 Refactor for
ParserExtensionConfig and...
Posted by Otto Fowler <ot...@gmail.com>.
Bump on this.
I would like to get this in before I take master again.
On September 2, 2017 at 23:28:45, ottobackwards (git@git.apache.org) wrote:
GitHub user ottobackwards opened a pull request:
https://github.com/apache/metron/pull/731
METRON-1146 Refactor for ParserExtensionConfig and Rest get all call
This a PR for some cleanup on the 1136 Feature Branch
- return a list from GET ( getAllParsers ) and not a map.
- fix names in Parser Extension Configs to be consistant with
extension/extensions
- add a field just called identifier, to be used instead of assembly name
- differentiate identifier/assemblyname
##
- [x] Is there a JIRA ticket associated with this PR? If not one needs to
be created at [Metron Jira](
https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
- [x] Does your PR title start with METRON-XXXX where XXXX is the JIRA
number you are trying to resolve? Pay particular attention to the hyphen
"-" character.
- [x] Has your PR been rebased against the latest commit within the target
branch (typically master)?
- [x] Have you ensured that the full suite of tests and checks have been
executed in the root metron
- [x] Have you written or updated unit tests and or integration tests to
verify your changes?
- [x] Have you verified the basic functionality of the build by building
and running locally with Vagrant full-dev environment or the equivalent?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ottobackwards/metron
ext-refactor-names-getall
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/731.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #731
----
commit 0d3b4c01fa41a3ee888a4debf249e62f82f2a184
Author: Otto Fowler <ot...@gmail.com>
Date: 2017-09-03T03:17:46Z
Refactor:
- return a list from GET ( getAllParsers ) and not a map.
- fix names in Parser Extension Configs to be consistant with
extension/extensions
- add a field just called identifier, to be used instead of assembly name
- differentiate identifier/assemblyname
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron issue #731: METRON-1146 Refactor for ParserExtensionConfig and Rest g...
Posted by anandsubbu <gi...@git.apache.org>.
Github user anandsubbu commented on the issue:
https://github.com/apache/metron/pull/731
Hi @ottobackwards , I guess you wanted to name the PR title as METRON-1136 instead of METRON-1146?
---
[GitHub] metron pull request #731: METRON-1146 Refactor for ParserExtensionConfig and...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron/pull/731#discussion_r137309525
--- Diff: metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/ExtensionServiceImpl.java ---
@@ -528,20 +527,26 @@ private void rollBackPatternsFromHdfs(InstallContext context) throws Exception{
private void writeExtensionConfiguration(InstallContext context) throws Exception {
ParserExtensionConfig config = new ParserExtensionConfig();
config.setParserExtensionParserName(context.extensionParserNames);
- config.setExtensionsBundleID(context.bundleID.get());
- config.setExtensionsBundleVersion(context.bundleVersion.get());
+ config.setExtensionIdentifier(context.extensionIdentifier.get());
+ config.setExtensionBundleID(context.bundleID.get());
+ config.setExtensionBundleVersion(context.bundleVersion.get());
config.setExtensionBundleName(context.bundleName.get());
config.setExtensionAssemblyName(context.extensionPackageName.get());
config.setDefaultParserConfigs(context.defaultParserConfigs.get());
- config.setDefaultEnrichementConfigs(context.defaultEnrichmentConfigs.get());
+ config.setDefaultEnrichmentConfigs(context.defaultEnrichmentConfigs.get());
config.setDefaultIndexingConfigs(context.defaultIndexingConfigs.get());
if(context.defaultElasticSearchTemplates.isPresent()) {
config.setDefaultElasticSearchTemplates(context.defaultElasticSearchTemplates.get());
}
- ConfigurationsUtils.writeParserExtensionConfigToZookeeper(context.extensionPackageName.get(),config.toJSON().getBytes(), client);
+ ConfigurationsUtils.writeParserExtensionConfigToZookeeper(context.extensionIdentifier.get(),config.toJSON().getBytes(), client);
--- End diff --
What is the logic for changing package name to identifier?
---