Skip to content

Conversation

@vitorarins
Copy link

Closes #27

@firminochangani
Copy link

Hey @vitorarins,
Perhaps it would interesting to extend the tests in order to cover the issue that you described in #27. cc: @m110

@amagow-ct
Copy link

Would there be any updated on this PR?
This would be a nice to have for my use case.

I am creating a message topic on runtime as topic.%s where %s is formatted at runtime. Because the library only allows setting the maxlen during init. I am not able to set the maxlen for my dynamic topic, causing memory issues in my redis store.

For the comment on extending tests @flowck, what would we be actually testing here?
The function options are getting passed to the redis publisher. Which should itself be tested in the redis-go library. Furthermore, the reason for this feature is to reduce the memory load on the store, which goes beyong the use of unit testing.
Thus, I believe it's not as useful to block this PR for the reason of having tests.

@minghsu0107 minghsu0107 merged commit e8f0c32 into ThreeDotsLabs:main Apr 26, 2024
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.

Feature request: add default Maxlens to Publisher config

4 participants