spec icon indicating copy to clipboard operation
spec copied to clipboard

Replace server.url with server.host

Open fmvilas opened this issue 4 years ago • 13 comments
trafficstars

Problem

Currently, AsyncAPI servers have a property called url. In many cases, we're showing examples of the url field as a host instead (without the protocol part of the URL) which is incorrect. On top of that, what's the point of having the protocol included in the url field if we already have the protocol field? This is an example of what I'm talking about:

asyncapi: 3.0.0
servers:
  localhost:
    url: mqtt://localhost
    protocol: mqtt
  production:
    url: kafka://my-production-server.com
    protocol: kafka

It can also create inconsistencies where the protocol in the URL and the protocol field don't match:

asyncapi: 3.0.0
servers:
  localhost:
    url: kafka://localhost
    protocol: mqtt

Proposal

My proposal is that we rename the url field as host, which would be more correct. Example:

asyncapi: 3.0.0
servers:
  localhost:
    host: localhost
    protocol: mqtt
  production:
    host: my-production-server.com
    protocol: kafka

We may also have to consider adding other parts of the URL like the path and query variables.

This is a proposal to fix #274

fmvilas avatar May 28 '21 21:05 fmvilas

Thanks for putting the issue together!

This is indeed a glitch of the existing spec (2.0.0) and I think the root cause of the glitch is the url field of the Server Object supporting both absolute and relative URLs. One cannot simply determine whether the url value is relative or not, as the scheme of the URL is expected to be defined in the separate protocol field.

The OpenAPI Server Object does not have a protocol field, which makes sense since the protocol can be inferred from the url field. If the url value is absolute, then it includes the protocol. If relative, then the protocol/scheme can be inferred from the URL that was used to access the OpenAPI document. This allows OpenAPI to support relative URLs, since the only protocol one can use is http (and https), which conveniently is the protocol though which one is serving their API documentation. However, this is not the case in the AsyncAPI world where in most scenarios: server protocol != API docs endpoint protocol.

So this proposal is essentially dropping support for relative server URLs.

dedoussis avatar May 31 '21 16:05 dedoussis

Instead of dropping the url field and introducing new fields (for the different parts of the url), I would suggest the inverse approach: Drop the protocol field and enforce the value of the existing url field to be an absolute URL.

The enforcement of the absolute url will provide support for all possible URL parts (even for niche parts such as fragment or userinfo)

To me, this feels like the most simple and future proof approach.

dedoussis avatar May 31 '21 16:05 dedoussis

🤔 Enforcing the URL to be always absolute would go against our plans to support any kind of APIs in the future (REST too).

Just thinking out loud: another option is that we decide to take both approaches and do something like "use url or protocol + host + path + ...". In other words, they'll be mutually exclusive. Not a fan of polymorphism here but definitely an option.

fmvilas avatar Jun 01 '21 08:06 fmvilas

Enforcing the URL to be always absolute would go against our plans to support any kind of APIs in the future (REST too).

I see. In that case, I think I lean towards your proposal: drop the URL field, and replace it with fields for all the possible URL parts.

dedoussis avatar Jun 10 '21 10:06 dedoussis

Happy to champion this RFC by the way. Will be putting together a Stage 1 proposal.

dedoussis avatar Jun 10 '21 12:06 dedoussis

This issue has been automatically marked as stale because it has not had recent activity :sleeping: It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation. Thank you for your contributions :heart:

github-actions[bot] avatar Aug 10 '21 00:08 github-actions[bot]

I think this makes sense to include in 3.0.0.

My only concern is that it becomes difficult for tooling to access the full URL. As you rarely need to access the individual components of the full URL, tooling would have to remember how to glue together the pieces of information.

Added a clarification issue for the parser-api to ensure we don't forget to solve this for tooling - https://github.com/asyncapi/parser-api/issues/37

jonaslagoni avatar Dec 17 '21 14:12 jonaslagoni

Is there any of you who wants to champion this? 🙂 Or can we consider this issue as needs champion? 🤔

jonaslagoni avatar Jan 19 '22 18:01 jonaslagoni

Anything addressing this issue should be required to also resolve #274, which basically stems from the fact that any bare host name is also syntactically a valid URI reference (e.g., in the absence of documented priority for interpreting the data, it is ambiguous whether url: api.example.com refers to "https://example.com/path-to-AsyncAPI-spec/api.example.com" or to "https://api.example.com/").

gibson042 avatar Jan 24 '22 13:01 gibson042

@dedoussis are you happy championing this one as you mentioned in the past?

smoya avatar Mar 21 '22 22:03 smoya

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

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 :heart:

github-actions[bot] avatar Jul 23 '22 00:07 github-actions[bot]

@fmvilas what about this one? Do we want to move it forward or rather stale it finally?

smoya avatar Jul 25 '22 10:07 smoya

I think this should be addressed in the next major version, otherwise, it will produce a breaking change. Or! We leave it for 4.0.0 and see how well the parser intent-driven API behaves 😄

fmvilas avatar Jul 29 '22 12:07 fmvilas

I made a proposal for this issue: https://github.com/asyncapi/spec/pull/888.

fmvilas avatar Dec 17 '22 10:12 fmvilas

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

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 :heart:

github-actions[bot] avatar Jun 04 '23 00:06 github-actions[bot]

Be patient, dear Github Actions. The PR is done and it will make it to 3.0.0. Just give us some more time :)

fmvilas avatar Jun 05 '23 09:06 fmvilas

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

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 :heart:

github-actions[bot] avatar Oct 24 '23 00:10 github-actions[bot]