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

Multiple hypens in stream names #57

Closed
dharmaturtle opened this issue Nov 4, 2020 · 2 comments
Closed

Multiple hypens in stream names #57

dharmaturtle opened this issue Nov 4, 2020 · 2 comments

Comments

@dharmaturtle
Copy link
Contributor

Hi! The code currently requires exactly one hyphen in a stream name. Is it reasonable to loosen that restriction? This would permit representing UUIDs in the canonical RFC-4122 format.

The readme implies this convention is from EventStore. Their category projections allow for some customizations, but notably do not require exactly one hyphen. In fact, when specifying the delimiting character, one must also specify whether to consider the first or last occurrence of the delimiter. Docs.

Some stream name examples just to demonstrate what it would look like:
account-40DC8220-D240-4530-847C-9F41E286C0A2
account-FDEED87B-A89A-40AC-90B4-E89FEE29454E_1A26E0BA-1826-4504-8183-C5EAD0693DB3

Of course, conventions being conventions, all this may be ineligible for change.

@bartelink
Copy link
Collaborator

For me, the default config of EventStore (be that first or last hyphen) should win, if >1 is to be permitted (even if these things are configurable).

My main intention is to give relatively clear guidance as to a sensible convention, more than it is to enforce it (always have a category name, use a different separator to split inside the 'aggregate id' portion of the stream name. The only real constraints are:

  • in Propulsion (and probably in Prometheus integrations for Equinox), being able to pull out a category from a streamName matters (and you shoiuld not have to enter into a big configuration exercise)
  • in EventStore, you're going to have a hard time in practice if you don't have a hyphen and/or if you don't use the default

There's no real problem with relaxing the restrictions, as long as:

  • there are sufficient tests (exactly one dash can be tested by eyeballing it)
  • the EventStore default is accepted / the standard (be that last or first dash)
  • the docs dont end up having to be as long as the EventStore ones ;)

Also, the message lies about the exact rule: https://github.com/jet/FsCodec/blob/master/src/FsCodec/StreamName.fs#L47 and https://github.com/jet/FsCodec/blob/master/src/FsCodec/StreamName.fs#L56 (the category is the bit before the first dash and >1 is handled/accepted due to the 2 being supplied to Split)

@dharmaturtle
Copy link
Contributor Author

I realized the code permits multiple hyphens in streamname identifiers, which is all I really care about. I opened up a PR about it, and we can continue this discussion there.

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

No branches or pull requests

2 participants