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

Dump command and require-table #248

Merged
merged 1 commit into from Jun 23, 2016
Merged

Dump command and require-table #248

merged 1 commit into from Jun 23, 2016

Conversation

HavokInspiration
Copy link
Member

The dump command should not set require-table to true when fetching the tables to add to the dump because it can lead to unexpected results as reported in #232 and #244.

It turned out the issue experienced by @ypnos-web and @dereuromark are because of this option.

@lorenzo As specified in #232, it should be known that tables from plugins will also be added to the dump when migrating or rolling back a set of migrations (basically, the system will now add every tables it finds in the dump), potentially creating a situations where tables from plugin will be added in diffs.
Maybe there could be an option in the migration diff command to exclude tables :

  • by specifying a list of table names
  • by specifying a plugin name. When used it will fetch all ORM table classes in the said plugin and exclude them from the diff.

We can keep the idea on the side right now and consider it if you have users reporting this issue. What do you think ?

@lorenzo
Copy link
Member

lorenzo commented Jun 22, 2016

I think this is a step in the right direction.

What happens if I set the require tables to true and also a plugin option? Would it save the dump in the plugin only for the tables in the plugin?

@HavokInspiration
Copy link
Member Author

HavokInspiration commented Jun 22, 2016

There is no require-table option on the dump command. But that can be added.

However, when a plugin option is found, whether or not there is a require-table option set and whatever its value, it will only fetch tables that have a Table class in order to really filter the table from the plugin. There is no better way to do it from my point of view.
That is valid for snapshots as well (this logic was first implemented for snapshots)

@lorenzo lorenzo merged commit b683dae into master Jun 23, 2016
@saeideng saeideng deleted the dump-require-table branch May 24, 2018 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants