fix: default empty string value properties #304
Conversation
When these environment variables are not set and do not have a default value in the replacement string - they remain set to their un-replaced value, which is never a valid configuration. This change adds a default value for: - env:S3_MINIO_ENDPOINT - env:S3_PATH_STYLE_ACCESS
|
|
@jdpgrailsdev @colesnodgrass can you take a quick look in this fix proposed by Justen? |
|
You can sort of test this fix without actually applying it by setting
|
There was a problem hiding this comment.
I think that @colesnodgrass may be working on cleaning some of this up too, so we should make sure that he takes a look as well.
There was a problem hiding this comment.
I recently went down a similar approach and was unable to get it to work, ultimately ended up having to fork the log4j2.xml file into a minio-free version.
You can set the environment-variable LOG4J_CONFIGURATION_FILE to tell java which file to use.
That being said, if this works for you, then I'm ok with this being merged in.
It's also work noting that a lot of these environment-variables around logging are currently being renamed and consolidated to help simplify our configuration settings.
|
👀 Running into this issue as well in a Helm environment and not sure how to dance around it atm. |
|
Your branch is not currently up-to-date with |
|
I'm going to close this PR, as I believe this issue has been fixed via a different approach. The The plan is to eventually migrate the log4j configuration out of xml and into code, but I am unsure when that will happen (definitely won't be until after |
|
Your branch is not currently up-to-date with |
What
The
log4j2.xmlconfiguration is missing a default value forS3_MINIO_ENDPOINTwhich is causing issues writing logs S3 Cloud Storage location.When these environment variables are not set and do not have a default value in the replacement string - they remain set to their un-replaced value, which is never a valid configuration.
When
S3_MINIO_ENDPOINTis set to an empty string, the replacement fails because the environment value is treated as unset; so the propertys3-minio-endpointgets set to literally${env:S3_MINIO_ENDPOINT}(since thesys:property is not set, and it uses this as the default value)ie: it becomes literally this, no substitution:
This causes problems down-stream for S3 since now technically both
s3-regionands3-minio-endpointis set; which is an invalid configuration, and results in a runtime exception:Related Isssues
Cannot create enum from ${env:S3_LOG_BUCKET_REGION}airbyte#34539How
This change adds a default value for:
Recommended reading order
airbyte-commons/src/main/resources/log4j2.xmlCan this PR be safely reverted / rolled back?
If you know that your PR is backwards-compatible and can be simply reverted or rolled back, check the YES box.
Otherwise if your PR has a breaking change, like a database migration for example, check the NO box.
If unsure, leave it blank.
🚨 User Impact 🚨
should be no breaking changes