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

Only dump schema after migrating in development environment. #37

Merged
merged 2 commits into from
Jul 23, 2013

Conversation

dividedmind
Copy link

Currently schema is always dumped after migration when the environment isn't a test environment. This is troublesome, because it means it's dumped in production; aside from not being useful, this breaks when the filesystem is read-only(-ish), like on Heroku. Ordinarily this is not obvious, but if you're running some scripts heroku-side you'll note that rake db:migrate returns nonzero error code. This patch fixes the issue by only dumping schema in development, arguably the only environment where it makes sense anyway.

@JonathanTron
Copy link
Member

Hi @dividedmind, sorry for taking so long to reply.

I get your point, but I'm not sure we should do anything about it.

I've encountered some scenarios with expectations that the schema.rb/structure.sql file will be generated in environments other than development.

I'm wondering if a better solution would not be to add a configuration for it, something like: exclude_environments_dump , with a default to [:test]. And then use this to determine if we should dump or not the schema/structure.

What do you think?

@dividedmind
Copy link
Author

I think it's simpler to add a sequel.schema_format = :none, and adjust the defaults accordingly.

Another option is to just add | Rails.env.production? to the original code. While I understand there can be environments outside of development that use dumping, if you're relying on schema dumping in production you're obviously doing something wrong.

@JonathanTron
Copy link
Member

I really like your idea of using sequel.schema_format = :none. I prefer to limit the hard-coded assumption about the environment (test is already a smell).

@dividedmind
Copy link
Author

Agreed :)

@dividedmind
Copy link
Author

Ah, wait, it won't work! We need schema_format for loading, too.

@dividedmind
Copy link
Author

How about this?

@JonathanTron
Copy link
Member

This looks awesome!

JonathanTron added a commit that referenced this pull request Jul 23, 2013
Only dump schema after migrating in development environment.
@JonathanTron JonathanTron merged commit 1307981 into TalentBox:master Jul 23, 2013
JonathanTron added a commit that referenced this pull request Jul 23, 2013
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.

2 participants