feat: Make disableMigration option handled by environment variable

[0417taehyun]

About the changes

Closes #

Important files

Discussion points

In some cases, people want to disable database migration. For example, some people or companies want to grant whole permissions to handle the schema by DBAs, not by application level hence I use parseEnvVarBoolean to handle disableMigration option by environment variable. I set the default value as false for the backward compatibility.

However, I wonder where to add test code of it because all the unit and e2e test is a kind of testing the whole configuration.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
unleash-monorepo-frontend	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 5, 2024 10:09am

[vercel]

@0417taehyun is attempting to deploy a commit to the unleash-team Team on Vercel.

A member of the Team first needs to authorize it.

[gastonfournier]

Hi @0417taehyun I think this makes sense because this is already a configurable option. There's a linting issue you should just run yarn lint:fix.

Regarding testing, we test that after the migrations applied Unleash works well, if migrations are not applied there's no guarantee that Unleash will work well and most likely it won't work, so I don't think there's need to test this. Maybe the only thing that comes to mind is testing that when this is disabled the migrations are not executed.

Maybe just checking if this log is written:

unleash/src/lib/server-impl.ts

Lines 146 to 147 in a7d581a

	if (config.db.disableMigration) {
	logger.info('DB migration: disabled');

But to be honest, I'm ok if you don't test this

[0417taehyun]

@gastonfournier

Thanks for answering my question and checking linting issue! I fixed the issued by using yarn lint:fix and check the DB migration: disabled log. I also agree that it is okay not to test about this logic. Thanks!

[gastonfournier]

Looks good! Thanks for the contribution!