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

docs: add database guide page #3704

Merged
merged 8 commits into from Feb 22, 2022
Merged

Conversation

maksimovicdanijel
Copy link
Contributor

Adds a guide's page for database integration. It focuses on officially maintained plugins (MySQL, Postgres, MongoDB, Redis, LevelDB).

Checklist

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Feb 16, 2022
Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

That's very good! Thanks.

Could you add a note to show that users can also create their plugins for any database library, for instance, knex ?

@luisorbaiceta
Copy link
Member

Great! This closes #3562

Copy link
Member

@jsumners jsumners left a comment

Choose a reason for hiding this comment

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

I think this is a good start, but as a reader I would expect more "guide" than "just install this plugin". In particular, it seems like the case of a missing database plugin could be expanded to a full section that details how one would write a new database plugin. It would cover things like how to decorate the instance properly and how to close the database connection when the Fastify server is stopped.

docs/Guides/Database.md Outdated Show resolved Hide resolved
connectionString: 'mysql://root@localhost/mysql'
})

fastify.get('/user/:id', (req, reply) => {
Copy link
Member

@jsumners jsumners Feb 16, 2022

Choose a reason for hiding this comment

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

Please do not use arrow functions for route handlers:

Suggested change
fastify.get('/user/:id', (req, reply) => {
fastify.get('/user/:id', function (req, reply) {

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Good work! Could you cover migrations too? I would recommend postgrator.

Comment on lines 192 to 198
function knexPlugin(fastify, options, done) {
if(!fastify.knex) {
fastify.decorate('knex', knex(options))
}

done()
}
Copy link
Member

Choose a reason for hiding this comment

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

This will keep a connection open and prevent fastify.close from working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, please take a look.


Fastify's ecosystem provides a handful of
plugins for connecting to various database engines.
This guide covers engines which have Fastify
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
This guide covers engines which have Fastify
This guide covers engines that have Fastify

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, please check.

This guide covers engines which have Fastify
plugins maintained within the Fastify organization.

> If a plugin for your database of choice doesn't exist
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
> If a plugin for your database of choice doesn't exist
> If a plugin for your database of choice does not exist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, please check.

@Fdawgs
Copy link
Member

Fdawgs commented Feb 17, 2022

Good work! Could you cover migrations too? I would recommend postgrator.

Good example of Postgrator being used would be covidgreen/covid-green-backend-api; ended up adopting it myself after seeing it here.

### Writing a plugin for a database engine

In this example, we will create a basic Fastify MySQL plugin
from scratch (it's stripped example, please use the official plugin in production).
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
from scratch (it's stripped example, please use the official plugin in production).
from scratch (it is a stripped-down example, please use the official plugin in production).

Copy link
Member

Choose a reason for hiding this comment

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

Technically, the informal language is okay as this is a "guide".

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

docs/Guides/Database.md Outdated Show resolved Hide resolved
Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

LGTM.

docs/Guides/Database.md Outdated Show resolved Hide resolved
docs/Guides/Database.md Outdated Show resolved Hide resolved
docs/Guides/Database.md Outdated Show resolved Hide resolved
docs/Guides/Database.md Outdated Show resolved Hide resolved
Co-authored-by: Frazer Smith <frazer.dev@outlook.com>
@mcollina mcollina merged commit 8b00394 into fastify:main Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants