Skip to content

feat: add timestamp variable#110

Merged
ali-ince merged 6 commits intoneo4j:mainfrom
ali-ince:add-timestamp-variable
May 15, 2024
Merged

feat: add timestamp variable#110
ali-ince merged 6 commits intoneo4j:mainfrom
ali-ince:add-timestamp-variable

Conversation

@ali-ince
Copy link
Contributor

@ali-ince ali-ince commented May 10, 2024

This PR adds message timestamp as a variable to cypher sink strategy.

@ali-ince ali-ince requested a review from a team as a code owner May 10, 2024 20:44
@@ -0,0 +1,7 @@
org.slf4j.simpleLogger.defaultLogLevel=info
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need logging by default?
could we turn it off, and only turn it on temporarily when troubleshooting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this is just applicable for tests, I think having it enabled would help troubleshooting in case of failures in CI as well - that's why I kept them enabled by default.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. I usually push against logs enabled by default, as test assertions are theoretically enough.
However, we're dealing here with a complex test infrastructure here, so I'm not against it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main reason for that is now we are using a random database and topic name per test case, and they are being logged through the logging infrastructure - which becomes quite important when you have a test failure.

@ali-ince ali-ince merged commit 1844cbc into neo4j:main May 15, 2024
@ali-ince ali-ince deleted the add-timestamp-variable branch May 15, 2024 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants