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

Move binding JSON schema files to main JSON schema repository #113

Closed
jonaslagoni opened this issue Jan 23, 2022 · 25 comments
Closed

Move binding JSON schema files to main JSON schema repository #113

jonaslagoni opened this issue Jan 23, 2022 · 25 comments
Labels
enhancement New feature or request

Comments

@jonaslagoni
Copy link
Member

Reason/Context

Currently, we store the binding JSON Schema files under each binding. However, it makes them hard to reach for others. Especially if we wish to provide them with our JSON Schema for the spec - https://github.com/asyncapi/spec-json-schemas/tree/master/schemas

See asyncapi/spec#507

Description

I suggest we move all the JSON Schema files for bindings to https://github.com/asyncapi/spec-json-schemas. That way we match the current setup for the specification and make it easy to reference them directly in the spec JSON Schema https://github.com/asyncapi/spec-json-schemas/blob/abeac12417603de5b5fd57be4af4c8289e274a2b/schemas/2.2.0.json#L773

@jonaslagoni jonaslagoni added the enhancement New feature or request label Jan 23, 2022
@dalelane
Copy link
Collaborator

Makes sense to me to keep it together. I'd be wary of deleting them from the bindings repo though, in case people are using URLs like https://raw.githubusercontent.com/asyncapi/bindings/master/kafka/json_schemas/message.json as $ref values - as we don't want to break anything.

Maybe a README in the json_schemas folders in the bindings repo explaining that the copies there are no longer being updated, and pointing at the correct new location?

I realise I'm being uber-cautious and conservative here! I certainly can't find any use of https://raw.githubusercontent.com/asyncapi/bindings/master/ from a quick search of public github repos, so I'm open to being told that I'm being over paranoid :-)

@derberg
Copy link
Member

derberg commented Jan 24, 2022

I opted in keeping them here when they were added to make contribution process easier -> one repo to push changes.

but we already have json schema separated from spec repo for a good reason, to easily bug fix errors in json schema without releasing the spec. So yeah, I think the best would be to move JSON Schemas from here to spec-json-schemas.

we just need to have it clear in readme and have clear guidelines for maintainers from this repo, to accept changes only if PR is opened against spec-json-schemas too

@jonaslagoni
Copy link
Member Author

jonaslagoni commented Jan 31, 2022

I get the cautiousness @dalelane, I didn't really think of that use case as there was no library to enable the usage I automatically dismissed they were being used 😆

We could just reference the files in spec-json-schemas using https://raw.githubusercontent.com/asyncapi/bindings/master/kafka/json_schemas/message.json. The bundler that is in place for asyncapi/spec-json-schemas#128 will automatically fetch the binding schemas and bundle them into the main schema.

However, we are not able to expose the bindings JSON Schema files in the website, asyncapi/website#502 (unless we want to maintain a package for bindings as well, but that seems like double the work for "nothing" 🤷). And we will still be using different tactics between the spec and bindings which I find confusing.

I still think it is the better option, as @derberg also points out 👍

@magicmatatjahu
Copy link
Member

magicmatatjahu commented Feb 23, 2022

Isn't it better to have the json schemas here and add a github workflow that would move all the JSON Schemas to the spec-json-schemas repository? Identical as we do with tsc_members.json in the community repository, we move that file to the website.

The bindings themselves currently live their own lives and have their own versioning which is not related to the specification versioning. Additionally, with the json schemas here you can easily set up champions for the given bindings who would accept and reject changes only for the given bindings. Forcing a contribution here and to spec-json-schemas would mean that Łukasz or Fran would have to accept it every time. Of course we can add champions in spec-json-schemas repository as well, but in my opinion this would only cause problems.

Similar approach we can introduce for extension registry. All definitions for extensions should be located in the extension-catalog and then we can move them by github workflow to the spec-json-schemas.

@derberg
Copy link
Member

derberg commented Mar 2, 2022

@magicmatatjahu most important is consistency imho, this is why I opt in for move to spec-json-schemas. Especially that we moved spec json schema away from spec repo for a reason. JSON Schemas should not be released together with spec but independently, they are considered to be a "tool". And bindings are spec.

@magicmatatjahu
Copy link
Member

It is a solution, but I look at it from the point of view of DX itself as someone who will contribute. The spec changes are not that much, so those 2 PRs can be done.

If we decide to move the JSON Schemas to a separate repo, then I'd like us to create a champion system in this repo and in spec-json-schemas.

@jonaslagoni
Copy link
Member Author

As it should right? I mean changes to the bindings are practically at the same level as the spec itself in my opinion. The only difference is that it targets a smaller section of the users.

It is a solution, but I look at it from the point of view of DX itself as someone who will contribute. The spec changes are not that much, so those 2 PRs can be done.

Yes, it is more confusing for someone who only wants to contribute to the bindings. However, if you contribute to bindings and then spec, it will in my opinion be more confusing 🤔

@magicmatatjahu
Copy link
Member

Yes, it is more confusing for someone who only wants to contribute to the bindings. However, if you contribute to bindings and then spec, it will in my opinion be more confusing 🤔

I don't know if we think the same but Binding spec !== AsyncAPI spec. Additionally, I think that we shouldn't hard bind JSON Schema of bindings to JSON Schema of spec (if you have such an idea) - we can have a huge problem with weight of given JSON Schema and it is important for frontend applications.

@jonaslagoni
Copy link
Member Author

I don't know if we think the same but Binding spec !== AsyncAPI spec
Yea, we don't 😆

Additionally, I think that we shouldn't hard bind JSON Schema of bindings to JSON Schema of spec (if you have such an idea) - we can have a huge problem with the weight of given JSON Schema and it is important for frontend applications.

But would you not always want to properly validate the bindings as well? Should not matter about the size of the library as that is a side effect you must accept.

@magicmatatjahu
Copy link
Member

@jonaslagoni I don't mean no binding validation but validation in a different way than through a single JSON Scheme AsyncAPI spec.

This is why we can have two versions of JSON Schema, one full and the other smaller without bindings. The first one can be used on the VSC side (as an example) and the second one can be used on our tools side. Take into account that someone can write a custom binding as an extension (we want to support such things) so we need to have a logic that would validate that binding, and having it, supporting validation of official bindings will be easy. Of course it should be discussed.

@jonaslagoni
Copy link
Member Author

Take into account that someone can write a custom binding as an extension (we want to support such things) so we need to have a logic that would validate that binding, and having it, supporting validation of official bindings will be easy

In my mind, that would not be binding, but an extension 🤔

Maybe it makes sense to continue this discussion on another issue? Is bindings part of the spec, or are they seen as extensions

@derberg
Copy link
Member

derberg commented Mar 17, 2022

so what, are we moving these or what?
and yes, definitely champion approach, with owners per binding definitely is a must.

if we are still in conflict on deciding where should files land, maybe we should try to simulate, and in general answer the question how bindings are released. This way we identify the flow and get answers if MD and JSON Schema can really be here together

@magicmatatjahu
Copy link
Member

If I may add something, I have no problem transferring the json schema of bindings to json-spec-schemas repo, but the json schema itself for bindings does not have the necessary information that we could read in the tools (parser-js) and validate properly. Most problematic gap as I see is no information for which versions of AsyncAPI a given binding can be used. We even have an issue opened for this - asyncapi/spec#590

I opt to go with direction like we want to go with extension, that we have "separate" spec for bindings/extensions by which we will describe bindings/extensions, something like this one example https://github.com/asyncapi/extensions-catalog#http-binding I know that we should rethinking extension "spec", but it will be better option that flat json-schemas. We have the time then we should use it.

@jonaslagoni
Copy link
Member Author

So how do we figure this one out? Cause it comes down to opposing viewpoints that cannot really be aligned, it's an either-or decision between the two approaches 😄

Should we do a live stream so we can discuss and clarify the pros and cons? 🤔

@magicmatatjahu
Copy link
Member

I will not be able to attend the 3.0.0 meeting today, but I still think that treating bindings as extensions is a better solution from the tooling and binding sharing point of view. Merging JSON Schema into one big schema is a solution we should create (e.g. for IDE), but we should also be able to import bindings as extensions. This is crucial, because people can create their own private bindings (private in company) and should also be able to validate them without weird tricks of merging the main JSON schema. Additionally, this will provide less weight to the specification itself, which is important for browser applications (Studio) to keep the application as small as possible.

I opt to have dedicating meeting for that topic.

@jonaslagoni
Copy link
Member Author

I opt to have dedicating meeting for that topic.

yea, let's do that 👍

Think it makes sense to do it once asyncapi/community#308 is merged, as the first ad hoc meeting :D

@derberg
Copy link
Member

derberg commented Apr 20, 2022

yeah let's have a call

the main difference is:

  • I opt in for small steps, do things one by one, so basically move files there first and then figure versioning
  • @magicmatatjahu sees a perfect world of bindings and extensions hold separately, supporting for the approach that people can have their own stuff. All of that is nice, but I do not know anyone actively working on that, so why should we slow down small steps 😄

@jonaslagoni
Copy link
Member Author

Added it to the agenda for the next 3.0 meeting. That can be our start to see if we need a dedicated meeting afterward.

@github-actions
Copy link

github-actions bot commented Sep 1, 2022

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added stale and removed stale labels Sep 1, 2022
@github-actions
Copy link

github-actions bot commented Jan 1, 2023

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@jonaslagoni
Copy link
Member Author

Still relevant 😄

@jonaslagoni
Copy link
Member Author

@GeraldLoeffler @rcoppen @SrfHead @lbroudoux @dalelane @whitlockjc @damaru-inc @CameronRushton @VisualBean @dpwdec @iancooper @KhudaDad414

Hey folks, sorry for the late ping, as I forgot to do so after asyncapi/spec-json-schemas#239 (comment) was merged.

As of #239, all bindings are now being validated alongside the spec and are no longer just for show.

This also means that as you are the codeowners of the different bindings, you will also become codeowners of the underlying schema files and spec-json-schemas repository. So you will continue to have full autonomy of the bindings you own 🙂

@derberg should send you an invite at some point soon (probably some time this week).

@derberg
Copy link
Member

derberg commented Oct 11, 2023

mentioning @mboss37 as well in regards to ☝🏼 comment from @jonaslagoni

@mboss37
Copy link
Collaborator

mboss37 commented Oct 12, 2023

@derberg @jonaslagoni Thanks for the info!

@jonaslagoni
Copy link
Member Author

As of the new bindings and spec-json-schema versions, we have successfully moved all JSON Schema files 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants