You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/05/27 08:50:15 UTC
[GitHub] [nifi] pvillard31 opened a new pull request #4299: NIFI-7490 - Add optional raw field to Syslog readers
pvillard31 opened a new pull request #4299:
URL: https://github.com/apache/nifi/pull/4299
Thank you for submitting a contribution to Apache NiFi.
Please provide a short description of the PR here:
#### Description of PR
_Enables X functionality; fixes bug NIFI-YYYY._
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with **NIFI-XXXX** where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `master`)?
- [ ] Is your initial contribution a single, squashed commit? _Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not `squash` or use `--force` when pushing to allow for clean monitoring of changes._
### For code changes:
- [ ] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi` folder?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] Have you verified that the full build is successful on JDK 8?
- [ ] Have you verified that the full build is successful on JDK 11?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE` file, including the main `LICENSE` file under `nifi-assembly`?
- [ ] If applicable, have you updated the `NOTICE` file, including the main `NOTICE` file found under `nifi-assembly`?
- [ ] If adding new Properties, have you added `.displayName` in addition to .name (programmatic access) for each of the new properties?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on pull request #4299: NIFI-7490 - Add optional raw field to Syslog readers
Posted by GitBox <gi...@apache.org>.
mattyb149 commented on pull request #4299:
URL: https://github.com/apache/nifi/pull/4299#issuecomment-641527834
+1 LGTM, verified the properties are there and work as expected. Thanks for the improvement! Merging to master
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on a change in pull request #4299: NIFI-7490 - Add optional raw field to Syslog readers
Posted by GitBox <gi...@apache.org>.
mattyb149 commented on a change in pull request #4299:
URL: https://github.com/apache/nifi/pull/4299#discussion_r433430043
##########
File path: nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/syslog/Syslog5424Reader.java
##########
@@ -59,18 +59,31 @@
@CapabilityDescription("Provides a mechanism for reading RFC 5424 compliant Syslog data, such as log files, and structuring the data" +
" so that it can be processed.")
public class Syslog5424Reader extends SchemaRegistryService implements RecordReaderFactory {
+
public static final String RFC_5424_SCHEMA_NAME = "default-5424-schema";
static final AllowableValue RFC_5424_SCHEMA = new AllowableValue(RFC_5424_SCHEMA_NAME, "Use RFC 5424 Schema",
"The schema will be the default schema per RFC 5424.");
+
+ static final String RAW_MESSAGE_NAME = "_raw";
+
public static final PropertyDescriptor CHARSET = new PropertyDescriptor.Builder()
.name("Character Set")
.description("Specifies which character set of the Syslog messages")
.required(true)
.defaultValue("UTF-8")
.addValidator(StandardValidators.CHARACTER_SET_VALIDATOR)
.build();
+ public static final PropertyDescriptor ADD_RAW = new PropertyDescriptor.Builder()
Review comment:
This property isn't added to the list of supported properties, so although it can be set for unit tests, it can't be set from the UI
##########
File path: nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/syslog/SyslogReader.java
##########
@@ -57,18 +57,31 @@
"Note: Be mindfull that RFC3164 is informational and a wide range of different implementations are present in" +
" the wild.")
public class SyslogReader extends SchemaRegistryService implements RecordReaderFactory {
+
public static final String GENERIC_SYSLOG_SCHEMA_NAME = "default-syslog-schema";
static final AllowableValue GENERIC_SYSLOG_SCHEMA = new AllowableValue(GENERIC_SYSLOG_SCHEMA_NAME, "Use Generic Syslog Schema",
"The schema will be the default Syslog schema.");
+
+ static final String RAW_MESSAGE_NAME = "_raw";
+
public static final PropertyDescriptor CHARSET = new PropertyDescriptor.Builder()
.name("Character Set")
.description("Specifies which character set of the Syslog messages")
.required(true)
.defaultValue("UTF-8")
.addValidator(StandardValidators.CHARACTER_SET_VALIDATOR)
.build();
+ public static final PropertyDescriptor ADD_RAW = new PropertyDescriptor.Builder()
Review comment:
Same here, the property isn't added to the list of supported property descriptors
##########
File path: nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/syslog/SyslogReader.java
##########
@@ -57,18 +57,31 @@
"Note: Be mindfull that RFC3164 is informational and a wide range of different implementations are present in" +
" the wild.")
public class SyslogReader extends SchemaRegistryService implements RecordReaderFactory {
+
public static final String GENERIC_SYSLOG_SCHEMA_NAME = "default-syslog-schema";
static final AllowableValue GENERIC_SYSLOG_SCHEMA = new AllowableValue(GENERIC_SYSLOG_SCHEMA_NAME, "Use Generic Syslog Schema",
"The schema will be the default Syslog schema.");
+
+ static final String RAW_MESSAGE_NAME = "_raw";
+
public static final PropertyDescriptor CHARSET = new PropertyDescriptor.Builder()
.name("Character Set")
.description("Specifies which character set of the Syslog messages")
.required(true)
.defaultValue("UTF-8")
.addValidator(StandardValidators.CHARACTER_SET_VALIDATOR)
.build();
+ public static final PropertyDescriptor ADD_RAW = new PropertyDescriptor.Builder()
+ .displayName("Raw message")
+ .name("syslog-5424-reader-raw-message")
+ .description("If true, the record will have a " + RAW_MESSAGE_NAME + " field containing the raw message")
+ .required(true)
+ .defaultValue("false")
+ .allowableValues("true", "false")
+ .build();
private volatile SyslogParser parser;
+ private volatile static boolean includeRaw;
Review comment:
This variable is never set so defaults to false, I think it needs a line like the Syslog5424Reader:
`includeRaw = context.getProperty(ADD_RAW).asBoolean();`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] ottobackwards commented on pull request #4299: NIFI-7490 - Add optional raw field to Syslog readers
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on pull request #4299:
URL: https://github.com/apache/nifi/pull/4299#issuecomment-636499100
+1 fwiw from me. this is a good add.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] pvillard31 commented on a change in pull request #4299: NIFI-7490 - Add optional raw field to Syslog readers
Posted by GitBox <gi...@apache.org>.
pvillard31 commented on a change in pull request #4299:
URL: https://github.com/apache/nifi/pull/4299#discussion_r433666463
##########
File path: nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/syslog/SyslogReader.java
##########
@@ -57,18 +57,31 @@
"Note: Be mindfull that RFC3164 is informational and a wide range of different implementations are present in" +
" the wild.")
public class SyslogReader extends SchemaRegistryService implements RecordReaderFactory {
+
public static final String GENERIC_SYSLOG_SCHEMA_NAME = "default-syslog-schema";
static final AllowableValue GENERIC_SYSLOG_SCHEMA = new AllowableValue(GENERIC_SYSLOG_SCHEMA_NAME, "Use Generic Syslog Schema",
"The schema will be the default Syslog schema.");
+
+ static final String RAW_MESSAGE_NAME = "_raw";
+
public static final PropertyDescriptor CHARSET = new PropertyDescriptor.Builder()
.name("Character Set")
.description("Specifies which character set of the Syslog messages")
.required(true)
.defaultValue("UTF-8")
.addValidator(StandardValidators.CHARACTER_SET_VALIDATOR)
.build();
+ public static final PropertyDescriptor ADD_RAW = new PropertyDescriptor.Builder()
+ .displayName("Raw message")
+ .name("syslog-5424-reader-raw-message")
+ .description("If true, the record will have a " + RAW_MESSAGE_NAME + " field containing the raw message")
+ .required(true)
+ .defaultValue("false")
+ .allowableValues("true", "false")
+ .build();
private volatile SyslogParser parser;
+ private volatile static boolean includeRaw;
Review comment:
good catch
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] pvillard31 commented on a change in pull request #4299: NIFI-7490 - Add optional raw field to Syslog readers
Posted by GitBox <gi...@apache.org>.
pvillard31 commented on a change in pull request #4299:
URL: https://github.com/apache/nifi/pull/4299#discussion_r433666340
##########
File path: nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/syslog/Syslog5424Reader.java
##########
@@ -59,18 +59,31 @@
@CapabilityDescription("Provides a mechanism for reading RFC 5424 compliant Syslog data, such as log files, and structuring the data" +
" so that it can be processed.")
public class Syslog5424Reader extends SchemaRegistryService implements RecordReaderFactory {
+
public static final String RFC_5424_SCHEMA_NAME = "default-5424-schema";
static final AllowableValue RFC_5424_SCHEMA = new AllowableValue(RFC_5424_SCHEMA_NAME, "Use RFC 5424 Schema",
"The schema will be the default schema per RFC 5424.");
+
+ static final String RAW_MESSAGE_NAME = "_raw";
+
public static final PropertyDescriptor CHARSET = new PropertyDescriptor.Builder()
.name("Character Set")
.description("Specifies which character set of the Syslog messages")
.required(true)
.defaultValue("UTF-8")
.addValidator(StandardValidators.CHARACTER_SET_VALIDATOR)
.build();
+ public static final PropertyDescriptor ADD_RAW = new PropertyDescriptor.Builder()
Review comment:
doh...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 closed pull request #4299: NIFI-7490 - Add optional raw field to Syslog readers
Posted by GitBox <gi...@apache.org>.
mattyb149 closed pull request #4299:
URL: https://github.com/apache/nifi/pull/4299
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org