-
Notifications
You must be signed in to change notification settings - Fork 18
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: implement FOREST_DISABLE_AUTO_SCHEMA_UPDATE and test for it #138
base: main
Are you sure you want to change the base?
Conversation
Hey @gonzalomolbar, and thanks for your contribution.
This is definitely something we tend to avoid. Not generating the schema could lead to important layout generation and permissions issues, and even outage in the usage of ForestAdmin, especially when using Developer Workflow/Production setup. Still, I'm curious to understand the underlying issue you are having with the fact that Thanks in advance 🙏 |
Hi @jeffladiray! No problem at all.
Mmm curious about this. It feels like it would have the same consequences as having the I guess the difference is that there is an actual command of the forest cli that can apply it (superseeding the setting) and not one for updating the schema file? Mmm. Not a big problem for us, since it's still an optional behaviour
It's just to keep a tighter control of the changes done to forest. Not every engineer that works in our org works on / is aware of how forest work. So even tho they do DB and model changes we prefer to have it changed in the schema only by the people that does. Also sometimes we've found actual git conflicts from 2 people updating something that is close in the forest schema, which had to be solved. Not a big deal but still Thanks for taking a look! let me know if it makes sense the setting proposal or not :P happy to do any changes to make it work if necessary |
Well, you are totally right (And I'm not a big fan of All your frontend/layout configuration on ForestAdmin depend on the
To give you an example of possible bad state, let's say you remove a column "foo" from table "Bar" in your business code. This is only one example, but there are a lot of others various things that could lead to this kind of issue.
Dropping the schema file & restarting the agent should be enough to cover this case, with no conflict handling needed - but still, I understand the frustration here. I'm pretty sure I'm missing something for your specific use-case though, or maybe that's related to some specifity in your development setup that I'm missing. We're still discussing your contirbution internally though, (As this would also need to be backported on all our agent if we were to validate this feature). In the meantime, and if this feature is a blocker for you, I'm pretty sure we can come up with a similar variable to force reset the schema file when you don't want it to be updated. Sorry for the long back and forth, but be sure you'll be updated once we've got some news to share. |
Yeah I was guessing the bad state would be somewhat around that direction. I've seen it happen by not applying it indeed, tho as u mention is kind of easier to fix that state in production and any non debug environments!
Oh yeah that,s personally what I did! No way you could sometimes figure out correctly what should go in or not in the conflicts! So indeed, I'd say it's not a big blocker tbh. If it's possible to have it in, I can see it as a weird but optional setting for some setups, but I'd understand if it's not wanted! Will keep an eye on the PR in case u have any update, but thanks for your insight! |
Definition of Done
FOREST_DISABLE_AUTO_SCHEMA_UPDATE
which, if true, will stop theforestadmin-schema.json
to be overwritten but still generating schema data for the app to function properlyDEBUG = true
self.assertRaises
in other tests in the file wich were not being used properlyNote
I didn't add anything in the changelog or versioning because I'm less familiar with it, and I'm not sure if you're the ones to add those changes when releasing it! So let me know if I should add anything
General
Security