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

Braid-Patch: why it is needed? #10

Closed
mitar opened this issue Oct 28, 2019 · 4 comments · Fixed by #20
Closed

Braid-Patch: why it is needed? #10

mitar opened this issue Oct 28, 2019 · 4 comments · Fixed by #20
Assignees

Comments

@mitar
Copy link
Member

mitar commented Oct 28, 2019

I do not get why we need Braid-Patch. I mean, I understand that it is elegant, but it is not yet-another-patch format with additional metadata about which synchronization algorithm to use. I do not get why those things have to be coupled together? Why simply we cannot say: my patch is in plain-text format, my patch is in JSON format, or my patch is in Braid-Patch-Simple format, which would be just:

bytes: [33:3889] = <binary-data>
json: .foo.bar[3].baz = {"1": {"two": "tree"}}
xml: /foo/bar/*[3]/baz = <one two="tree"/>

This is all information you have there really, decoupled from the synchronization algorithm. So why couple it together. Once represented this way we can also observe some other issues with the spec:

  • Where are standard values for selector type (e.g., bytes) and how they map to selectors (e.g., [33:3889]) defined? It looks like there is some hidden assumption that it is known that bytes should use [33:3889] should use .foo.bar[3].baz. If I am oblivious, what are those? Is that Xpath? JSON pointer? I have no idea.
  • What are valid combination of selector types and content types. Or how does one know which one they should implement in their Braid-compatible implementation? So if I want to put a label on my system saying it is Braid-compatible, which combinations I should support? This brings back that it is pretty unclear how that JSON - XML conversion can be done (JSON - XML conversion #4) so it is important also to know which combinations are supported, when they are maybe unexpected.

(BTW, not sure why are you talking about content types in this spec, you probably want to talk about Media Types, content type is a header in HTTP.)

So, to go back to the main issue here. So I think this should be split into two: one is a new patch format. And another is some standard way to specify/register/index different synchronization algorithms and their parameters. So in HTTP world, we might want for the latter to have something like Synchronization-Type: sync9(array). But I do not think why I could not have Synchronization-Type: sync9(array) with Content-Type: application/json-patch+jsonand then I would not have to use Braid-Patch. I see the benefit of simplicity of Braid-Patch, but I also see the benefit of simply reusing existing patch formats. I see those formats as a strict subset of Braid-Patch, so they should be compatible. But why to require to use Braid-Patch just to be able to also specify the indented synchronization type.

@mitar
Copy link
Member Author

mitar commented Oct 29, 2019

If it is OK, I can make an attempt at splitting it into two specs?

@toomim
Copy link
Member

toomim commented Oct 29, 2019

That's certainly ok! Be sure to add your name to the authors list at top and bottom.

This would help to get a more concrete idea of what you are thinking.

@toomim
Copy link
Member

toomim commented Oct 29, 2019

Conclusion from today's meeting:

  • Mitar's issue is that Braid-Patch right now is the only way to re-use a synchronization algorithm. It would be nice to be able abstract that outside of Braid-Patch, so that we could re-use a synchronizer even if we don't want to use the same format for patches.

@mitar
Copy link
Member Author

mitar commented Oct 30, 2019

I am marking this as actionable in the sense that I will make a PR and then we can see the concrete proposal here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants