Add support for Neo4j configuration #992
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
fbiville
force-pushed
the
neo4j/envvar_support
branch
from
8fb1e47 to
b8f0b80
Compare
modules/neo4j/config.go
Outdated
| @@ -11,6 +12,8 @@ type config struct { | |||
| imageCoordinates string | |||
| adminPassword string | |||
| labsPlugins []string | |||
| neo4jSettings map[string]string | |||
| stderr io.Writer | |||
There was a problem hiding this comment.
What's the intention of this writer? Testcontainers provides with a Logger interface that could be passed as field of the GenericContainerRequest struct (see https://github.com/testcontainers/testcontainers-go/pull/992/files#diff-cdacf25279bf22e2de635c654d0bb2c37c1b5699d8c13c07da3c0886b2087218R72-R75)
You could define the logger elsewhere and attach it to the container and use it when validating the config.
There was a problem hiding this comment.
ah cool, I was looking for a logger yesterday and could not find it. Will use the logger then
There was a problem hiding this comment.
I'm thinking more and more that:
- either the module does not validate conflicting settings at all
- or report validation errors just before the container is created
- or fails hard right away at the first conflict
Eagerly logging a warning introduces a configuration ordering issue (setting a custom logger after some settings are set means that custom logger may miss warning messages)
fbiville
force-pushed
the
neo4j/envvar_support
branch
from
3a2b7f2 to
92fb5b8
Compare
fbiville
requested review from
eddumelendez and
mdelapenya
and removed request for
eddumelendez and
mdelapenya
|
@fbiville I've started seen that every CI build fails for the Neo4j module, and cannot understand the reason. They fail even for PRs that were not modified and that job was re-triggered. It's weird that I run the tests locally and they pass. Do you know why? 🙏 |
|
I noticed that the GH worker changed:
|
|
For some reason, the inconsistency is no longer present 🤷 👻 |
fbiville
force-pushed
the
neo4j/envvar_support
branch
from
e58d928 to
82e1e65
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
* Add support for Neo4j configuration * Remove testify * Add missing setting name example in docs * Use logger * Document possible config ordering issue * Remove bogus license header
What does this PR do?
This PR adds support for customizing the Neo4j configuration before Neo4j starts.
Why is it important?
This is useful for many things: customizing logging, connectivity, the JVM used by Neo4j, etc etc
Related issues
Follows up after the initial implementation of #921.
Follow-ups
I'm not entirely sure yet whether conflicting setting keys should fail hard or not.
The initial implementation just logs a message when that happens.