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

Replace server.url with server.host #547

Closed
fmvilas opened this issue May 28, 2021 · 17 comments
Closed

Replace server.url with server.host #547

fmvilas opened this issue May 28, 2021 · 17 comments
Labels
💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md)

Comments

@fmvilas
Copy link
Member

fmvilas commented May 28, 2021

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 fmvilas added the 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md) label May 28, 2021
@dedoussis
Copy link

dedoussis commented May 31, 2021

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
Copy link

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.

@fmvilas
Copy link
Member Author

fmvilas commented Jun 1, 2021

🤔 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.

@dedoussis
Copy link

dedoussis commented Jun 10, 2021

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
Copy link

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

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴
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 ❤️

@jonaslagoni
Copy link
Member

jonaslagoni commented Dec 17, 2021

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 - asyncapi/parser-api#37

@jonaslagoni
Copy link
Member

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

@gibson042
Copy link
Contributor

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/").

@smoya
Copy link
Member

smoya commented Mar 21, 2022

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

@github-actions
Copy link

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 the stale label Jul 23, 2022
@smoya
Copy link
Member

smoya commented Jul 25, 2022

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

@github-actions github-actions bot removed the stale label Jul 26, 2022
@fmvilas
Copy link
Member Author

fmvilas commented Jul 29, 2022

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
Copy link
Member Author

fmvilas commented Dec 17, 2022

I made a proposal for this issue: #888.

@github-actions
Copy link

github-actions bot commented Jun 4, 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 ❤️

@github-actions github-actions bot added the stale label Jun 4, 2023
@fmvilas
Copy link
Member Author

fmvilas commented Jun 5, 2023

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

@github-actions github-actions bot removed the stale label Jun 25, 2023
@github-actions
Copy link

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 the stale label Oct 24, 2023
@smoya smoya removed the stale label Oct 25, 2023
@smoya smoya closed this as completed Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md)
Projects
None yet
Development

No branches or pull requests

6 participants