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

Create conventions #211

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Create conventions #211

wants to merge 2 commits into from

Conversation

dpwdec
Copy link
Collaborator

@dpwdec dpwdec commented Jul 3, 2023

Description

  • ...
  • ...
  • ...

Related issue(s)

Co-authored-by: iancooper <[email protected]>
@asyncapi-bot
Copy link
Contributor

asyncapi-bot commented Jul 3, 2023

We require all PRs to follow Conventional Commits specification.
More details 👇🏼

 No release type found in pull request title "Create conventions". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

dpwdec added a commit to dpwdec/bindings that referenced this pull request Jul 3, 2023
iancooper added a commit to iancooper/bindings that referenced this pull request Jul 3, 2023
Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great initiative! Left some suggestions.

Comment on lines +15 to +17
Prefer use of the [Bindings Extensions](https://github.com/asyncapi/extensions-catalog) format when defining a binding over raw JSON Schema.

The Binding Extensions format works with tools because it indicates where in the specification the binding should be used via a Hook. Using a Hook signals clearly to the user of an extension on which of Channel, Operation or Message the binding can be used. Implementers may choose to use author a binding so that the only fields on the binding are those appropriate to the object identified by the Hook. It also makes it possible for tools to determine correctness of an AsyncAPI file by ensuring that the binding is only used where permitted.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the "bindings extension format" will ever exist. This was a proof of concept that I did before we had bindings themselves. We should probably not recommend this here.


The Binding Extensions format works with tools because it indicates where in the specification the binding should be used via a Hook. Using a Hook signals clearly to the user of an extension on which of Channel, Operation or Message the binding can be used. Implementers may choose to use author a binding so that the only fields on the binding are those appropriate to the object identified by the Hook. It also makes it possible for tools to determine correctness of an AsyncAPI file by ensuring that the binding is only used where permitted.

### **Item** 2 Prefer JSON for Bindings
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### **Item** 2 Prefer JSON for Bindings
### **Item 2** Prefer JSON for Bindings

There may be multiple SDKs for a particular protocol, for different languages and frameworks. In many cases the variables required by these targets remain common across the targets and can simply be inferred from the protocol. In this case it is possible to define a common set of fields for a binding that can be used by Scaffolding regardless of the SDK. In other cases, SDKs vary considerably for the middleware. In this case you may prefer to provide a binding for the middleware that facilitates code generation for that middleware.

### **Item 5** Assume Separate Endpoints Have Separate AsyncAPI Files
Producers and consumers are usually separate apps, they may belong to separate teams, and under models such as pub-sub, there may an architectural goal to decouple producer and consumer by ensuring that each knows about the channel, but not the other. For this reason, you should assume that it will be common to define separate endpoints for the producer and consumer, but that the channel and the message schema will be shared. (Users may use $ref to prevent duplication, but that does not change these guidelines).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Producers and consumers are usually separate apps, they may belong to separate teams, and under models such as pub-sub, there may an architectural goal to decouple producer and consumer by ensuring that each knows about the channel, but not the other. For this reason, you should assume that it will be common to define separate endpoints for the producer and consumer, but that the channel and the message schema will be shared. (Users may use $ref to prevent duplication, but that does not change these guidelines).
Producers and consumers are usually separate apps, they may belong to separate teams, and under models such as pub-sub, there may be an architectural goal to decouple producer and consumer by ensuring that each knows about the channel, but not the other. For this reason, you should assume that it will be common to define separate endpoints for the producer and consumer, but that the channel and the message schema will be shared. (Users may use $ref to prevent duplication, but that does not change these guidelines).

Comment on lines +194 to +198
Where a protocol defines metadata that forms part of the message sent via middleware it should be defined as part of the header schema (potentially as MessageTraits), and where the protocol defines data (or transmits metadata in the payload) it should be defined as part of the payload. Note that the payload does not support MessageTraits.

Tooling to validate that messages match the schema will expect to verify against the schema of the message payload and headers, putting message metadata into a Message Binding makes this harder as tooling must know to look there as well.

Implementers of producers or consumers are also likely to assume that the the can refer to #/Components/Schema to understand payloads and headers, and adding metadata into bindings in addition makes it harder to comprehend the schema of a message.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not true. The right place to set protocol-specific message properties is actually the bindings, not traits or headers. See the definition of the Message headers field:

Schema definition of the application headers. Schema MUST be of type "object". It MUST NOT define the protocol headers.

The reason this is separated is to prevent mixing protocol-specific and application-specific metadata in a single place and the user having to define the shape of the metadata with JSON Schema every time.

My suggestion is that we remove the whole Item 9.

```

### **Item 10** Be Consistent with Other Protocols
Design your binding with the possibility that a producer might expose a message over different bindings - so as to support a wider range of consumers. By following the guidelines in this document your binding should be compatible with other bindings, and simplify the development of tooling that supports those because they operate at the same level.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

your binding should be compatible with other bindings

How exactly can someone accomplish this? We don't have a common structure among bindings yet.

Copy link

github-actions bot commented Nov 5, 2023

This pull request 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 pull request, add a comment with detailed explanation.

There can be many reasons why some specific pull request 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 pull request forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Nov 5, 2023
@smoya
Copy link
Member

smoya commented Nov 6, 2023

@dpwdec Is this PR supposed to be a replacement for #75 ?

Copy link

github-actions bot commented Mar 6, 2024

This pull request 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 pull request, add a comment with detailed explanation.

There can be many reasons why some specific pull request 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 pull request forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Mar 6, 2024
@smoya
Copy link
Member

smoya commented Mar 11, 2024

ping @dpwdec :D

@github-actions github-actions bot removed the stale label Mar 12, 2024
Copy link

This pull request 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 pull request, add a comment with detailed explanation.

There can be many reasons why some specific pull request 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 pull request forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants