Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Enhance the monitor verb syntax #301

Merged
merged 5 commits into from
Mar 15, 2023
Merged

feat: Enhance the monitor verb syntax #301

merged 5 commits into from
Mar 15, 2023

Conversation

gkc
Copy link
Contributor

@gkc gkc commented Mar 15, 2023

- What I did
feat: Enhanced the monitor verb syntax (a) to allow client to request that only regex-matching notifications are sent - e.g. do not send other 'control' type notifications like the 'statsNotifications' (b) to allow client to indicate that this socket is also being used for request-response interactions

- How I did it

  • added strict and multiplexed flags to the monitor verb syntax regex
  • split the monitor regex over multiple lines for readability
  • added documentation of the monitor regex
  • added strict and multiplexed flags to the MonitorVerbBuilder, defaulting to false
  • added getBuilder(String command) method to MonitorVerbBuilder so can generate a builder from a command and thus do round-trip tests to ensure nothing is being lost
  • improved documentation of the MonitorVerbBuilder members and methods
  • Added unit tests for MonitorVerbBuilder for the various combinations of including round-trip tests from builder to command to builder to command

- How to verify it
Tests pass

gkc added 3 commits March 15, 2023 13:29
…that only regex-matching notifications are sent - e.g. do not send other 'control' type notifications like the 'statsNotifications' (b) to allow client to indicate that this socket is also being used for request-response interactions

test: Add unit tests for MonitorVerbBuilder including round-trip tests from builder to command to builder to command
@gkc gkc requested a review from cpswan March 15, 2023 13:45
@gkc
Copy link
Contributor Author

gkc commented Mar 15, 2023

Once this PR is closed then we can start working on implementation in the atServer.

@cpswan I settled on strict and multiplexed as the names of the new flags for the monitor verb. If you have better names, please suggest them better before we set them in stone

gkc added 2 commits March 15, 2023 13:56
style: Removed an unnecessary use of toString() in MonitorVerbBuilder.buildCommand
@cpswan
Copy link
Member

cpswan commented Mar 15, 2023

@gkc gkc merged commit 4cd9790 into trunk Mar 15, 2023
@gkc gkc deleted the gkc-enhance-monitor-verb branch March 15, 2023 17: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