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

feat: expose all AsyncAPI JSON Schema documents #502

Closed

Conversation

jonaslagoni
Copy link
Member

@jonaslagoni jonaslagoni commented Dec 9, 2021

Description
This PR exposes all the AsyncAPI JSON Schema documents so they are fetchable from the path http://asyncapi.com/definitions/<version>/<name>.json.

Related issue(s)
Blocked by asyncapi/spec-json-schemas#128

@netlify
Copy link

netlify bot commented Dec 9, 2021

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 4ac90b4
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/62555fd2d0f46000089ab792
😎 Deploy Preview https://deploy-preview-502--asyncapi-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

const { document, version } = req.query
try {
// eslint-disable-next-line no-undef
const schema = require(`@asyncapi/specs/definitions/${version}/${document}`)
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 this is going to work. Our website is not running server-side code but only compiled/bundled HTML+CSS+JS. Not sure if Netlify offers the possibility to have server-side code either.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, our website is completely static. We build it and Netlify takes the result and serves it statically.

Copy link
Member Author

Choose a reason for hiding this comment

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

Think I have a workaround by utilizing the build scripts.

Copy link
Member

Choose a reason for hiding this comment

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

Yup, that's the only way. We do everything at build time.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is honestly also the safest solution 😄

Copy link
Member

Choose a reason for hiding this comment

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

Yup, that's the only way. We do everything at build time.

It is honestly also the safest solution 😄

Remember we need then to trigger a new build every time we merge a PR that changes the schemas at https:/asyncapi/spec-json-schemas

Copy link
Member Author

Choose a reason for hiding this comment

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

Will be done automatically, because our CI system update the package versions and create a new release 🙂

Copy link
Member

Choose a reason for hiding this comment

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

That's right 👍

BTW, there is another way for making this happen. Using Netlify rewrites so URL's can be redirected (proxied) to GH raw files, meaning we won't need to care about compiling the schemas here but just Netlify will grab the content from GH raw files (using caching mechanisms provided by GH CDN). It is as easy as this example shows https://docs.netlify.com/routing/redirects/rewrites-proxies/#proxy-to-another-service

The cons is that if we ever change the branch name, directory names or shape, those links will be broken 😓 .
Just to let you know it is a thing as well!

@jonaslagoni
Copy link
Member Author

jonaslagoni commented Dec 9, 2021

The files will not be visible until the other PR is merged and package updated.

@jonaslagoni jonaslagoni marked this pull request as ready for review March 14, 2022 12:42
fmvilas
fmvilas previously approved these changes Mar 14, 2022
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.

LGTM 👍

@jonaslagoni
Copy link
Member Author

This works locally, but I can't seem to access the resources here: https://deploy-preview-502--asyncapi-website.netlify.app/definitions/2.0.0/asyncapi.json

Any idea if this is only for previews, or if this is how it will be deployed? 🤔

@@ -31,6 +31,7 @@
},
"homepage": "https:/asyncapi/website#readme",
"dependencies": {
"@asyncapi/specs": "^3.0.0-next-major.1",
Copy link
Member

Choose a reason for hiding this comment

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

Why 3.0.0? Should we just use the latest stable version?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the introduction of split schemas and bundling them is a breaking change, i.e. version 3.0.0 of the library :)

Copy link
Member

Choose a reason for hiding this comment

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

Ouch, yeah, sorry. 😅

@smoya
Copy link
Member

smoya commented Mar 14, 2022

Any idea if this is only for previews, or if this is how it will be deployed? 🤔

Netlify deploy previews are just normal deploys so whatever you see in the preview is what you will see in the final deploy.
I guess there is something wrong on the build.

@jonaslagoni
Copy link
Member Author

Any idea if this is only for previews, or if this is how it will be deployed? 🤔

Netlify deploy previews are just normal deploys so whatever you see in the preview is what you will see in the final deploy. I guess there is something wrong on the build.

Does netlify automatically ignore gitignored files? 🤔 https:/asyncapi/website/blob/2f4de3ab0c292c30605887ac7e0451b9dbf831e5/.gitignore

For example, I can't access the rss file either https://deploy-preview-502--asyncapi-website.netlify.app/rss.xml

@jonaslagoni
Copy link
Member Author

jonaslagoni commented Mar 14, 2022

Seems like my production build does include the files... So they are not ignored.
billede

Is there any limitation as to which file extensions are allowed to be accessed maybe? 🤔 cc @magicmatatjahu

@smoya
Copy link
Member

smoya commented Mar 14, 2022

Is there any limitation as to which file extensions are allowed to be accessed maybe?

Not in Netlify side. I built up this test repo: https:/smoya/test-json-netlify. See the deploy https://gifted-bassi-75f047.netlify.app/.

Maybe at JS router level? however this should be reproducible locally.

@smoya
Copy link
Member

smoya commented Mar 14, 2022

Does netlify automatically ignore gitignored files? 🤔

AFAIK, Netlify doesn't use .gitignore file for skipping files to be uploaded. Everything in the publish directory is uploaded.

@smoya
Copy link
Member

smoya commented Mar 14, 2022

@jonaslagoni As I can see, the files were never uploaded to Netlify: https://app.netlify.com/sites/asyncapi-website/deploys/622f368a43a8150008039854#L6686-L6800 Not sure that's the whole dir list. Just the ones with possible file optimizations I guess

@jonaslagoni
Copy link
Member Author

Now it works 🧐

https://deploy-preview-502--asyncapi-website.netlify.app/definitions/2.0.0/asyncapi.json

* Copy all the internal schema files to expose them as static files.
*/
module.exports = function copySchemaFiles() {
const sourcePath = path.resolve(__dirname, '../node_modules/@asyncapi/specs/definitions');
Copy link
Member

Choose a reason for hiding this comment

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

I would add a console.log somewhere so you can at least have some logs of what is happening

@derberg
Copy link
Member

derberg commented Mar 17, 2022

I have to say I'm not a fan of this.
We have some related discussions here #780

@smoya
Copy link
Member

smoya commented Mar 18, 2022

I have to say I'm not a fan of this. We have some related discussions here asyncapi/website#780

What is the thing you are not a fan of exactly? Just to understand it better.

@derberg
Copy link
Member

derberg commented Mar 18, 2022

@smoya exposing files on the current website, as static content as the rest of the website, with zero analytics plugged in.

@smoya
Copy link
Member

smoya commented Mar 18, 2022

@smoya exposing files on the current website, as static content as the rest of the website, with zero analytics plugged in.

There are solutions exposed in the other PR worth evaluating. For sure the best for serving any static content are CDN's and we do use Netlify already. We should keep the discussion in there indeed 👍.

@jonaslagoni
Copy link
Member Author

@smoya exposing files on the current website, as static content as the rest of the website, with zero analytics plugged in.

Isn't analytics a whole other beast not really related to this? It's something that can always be included once an agreement is in order how to handle it 🤔

@smoya
Copy link
Member

smoya commented Mar 18, 2022

@smoya exposing files on the current website, as static content as the rest of the website, with zero analytics plugged in.

Isn't analytics a whole other beast not really related to this? It's something that can always be included once an agreement is in order how to handle it 🤔

The point is that @derberg suggested that, in an effort to measure the adoption of AsyncAPI, to:

storing somewhere info whenever JSON Schema is fetched by users, so we can count it as "usage"

Which I think we all agree is completely useful.
The thing is that if we use a CDN for storing static files, we should ensure we can have those metrics somehow; be mindful the URL for any JSON Schema will hit directly the CDN, the idea is to avoid any intermediate server (at least handled by us).
Whatever solution we choose here in terms of CDN, it should come with the metrics we need (which Netlify seems to have).

So it's not something you can't include later on. @derberg could we do a quick test on Netlify metrics from AsyncAPI account? Just to see if the metrics it provides are enough for our purpose.

@derberg
Copy link
Member

derberg commented Apr 5, 2022

We got access to Netlify Analytics for free:

app netlify com_sites_asyncapi-website_analytics

Also look at my conversation with support:

support:

Hey Lukasz,
 
Functions are already available for free for the first 125k invocations in your billing month.
 
About Analytics, we can enable it for free on 1 site on the open source plan. However, I'm not sure if it will serve your needs. Analytics is not as robust as you seem to expect from your descriptions. More specifically, Analytics counts only text/html pages and not which file was fetched how many times. Do you still want us to enable it?

me:

Thanks a lot for fast response. So just to clarify, if we have on CDN also png files, json files, and other NON-HTML files, fetch requests are not counted?

Could we also anyway get it enabled for "asyncapi-website" site? cause if you really support only html, we will explore possible workarounds.

cheers,
Lukasz

support:

	

Hrishikesh Kokate (Netlify)

Mar 18, 2022, 8:13 AM EDT

Hey Lukasz,
 
That is correct. While we count those requests in our backend, while showing analytics, we filter traffic for "pages" of the site instead of assets.

so the only option to do analytics is to have a proxy between client and CDN, or simply have no CDN and store schemas on a server side

@derberg
Copy link
Member

derberg commented Apr 12, 2022

@jonaslagoni @smoya @fmvilas @magicmatatjahu

did you have a chance to see my info about Netlify Analytics. IMHO we stay with option to have all JSON files in server-api that would work like a proxy to do analytics. It is up to server-api maintainers to decide if it is ok to first do it in server-api and then if because of the load, it should be split. Nevertheless, IMHO JSON files should not be exposed directly on the website here as we are looking an opportunity to track adoption

@jonaslagoni
Copy link
Member Author

did you have a chance to see my info about Netlify Analytics. IMHO we stay with option to have all JSON files in server-api that would work like a proxy to do analytics. It is up to server-api maintainers to decide if it is ok to first do it in server-api and then if because of the load, it should be split. Nevertheless, IMHO JSON files should not be exposed directly on the website here as we are looking an opportunity to track adoption

I think it is a great idea, I just don't think that is where we should start, especially since this PR is ready 🤷 Removing the package from the website once the server-api can handle it with analytics should not be any problems, right? As long as the server-api can handle http://asyncapi.com/definitions/1.0.0/asyncapi.json routes.

@github-actions
Copy link

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 52
🟠 Accessibility 74
🟢 Best practices 92
🟢 SEO 90
🔴 PWA 30

Lighthouse ran on https://deploy-preview-502--asyncapi-website.netlify.app/

@smoya
Copy link
Member

smoya commented Apr 13, 2022

did you have a chance to see my info about Netlify Analytics. IMHO we stay with option to have all JSON files in server-api that would work like a proxy to do analytics. It is up to server-api maintainers to decide if it is ok to first do it in server-api and then if because of the load, it should be split. Nevertheless, IMHO JSON files should not be exposed directly on the website here as we are looking an opportunity to track adoption

I think it is a great idea, I just don't think that is where we should start, especially since this PR is ready 🤷 Removing the package from the website once the server-api can handle it with analytics should not be any problems, right? As long as the server-api can handle http://asyncapi.com/definitions/1.0.0/asyncapi.json routes.

That's right @jonaslagoni. If needed, we can later add a Netlify Rewrite rule in case we move those files to other place.

However, I'm wondering if we should host our files under a subdomain rather than the apex, like https://definitions.asyncapi.com/1.0.0.json.
The reason this is in my head is because in case we move those files to other place out of Netlify, we would just need to change DNS instead of adding such a Netlify redirect rule, meaning we will remove one extra hop (Netlify).

@smoya
Copy link
Member

smoya commented Apr 14, 2022

@jonaslagoni I gave another thought on my comment about using Netlify rewrite rules to serve the content direct from GH (i.e. https://raw.githubusercontent.com/asyncapi/spec-json-schemas/master/schemas/2.3.0.json).

I think it is a super simple solution that only requires adding one rewrite rule into netlify.toml and all will work from there. There is cool benefit on this solution: the schema files will be always served from files in spec-json-schemas master branch, so we won't need to manually trigger an update on any package on every release.

[[redirects]]
  from = "/definitions/*"
  to = "https://raw.githubusercontent.com/asyncapi/spec-json-schemas/master/schemas/:splat"
  status = 200

This will proxy (not redirect) anything on path /definitions/* to https://raw.githubusercontent.com/asyncapi/spec-json-schemas/master/schemas/{rest of the path}.

I created a simple project as a demo here.
You can see there are no files hosted anywhere, just one netlify.toml.

If you visit https://silly-stroopwafel-1e879a.netlify.app/definitions/2.3.0.json, the content is served directly from https://raw.githubusercontent.com/asyncapi/spec-json-schemas/master/schemas/2.3.0.json.

We could either add this to the current website netlify.toml file or rather, create a new site in Netlify by just adding that netlify.toml to the spec-json-schemas. Depending on what solution we finally go regarding the domain name VS subdomain (I suggested in my previous comment)

@jonaslagoni
Copy link
Member Author

We are dropping this solution and going with @smoya solution, much more clean 👍

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

Successfully merging this pull request may close these issues.

5 participants