Design Meeting Notes - November 29, 2012
Handling pull requests
We currently have a pull request for which most of the code is good but for which there are a few issues. This raises two questions:
- In general, should we just accept the request and make the fixes before checking in? This is what we have done up to now.
- The team felt that we should ask the contributor to make the changes in the same way that we would ask an internal developer to make the changes when doing an internal code review. This will help reinforce the principle that we have a high quality bar for
- Specifically, in this case, there are some interfaces that are used for mocking but don’t appear to add much value—the single implementations of these classes could be mocked anyway, and these are not interfaces where we expect there will ever be different
implementations, but it could be argued that having them is a good practice. Should we push back on this?
- After looking at the interfaces the team felt that they didn’t add enough value and that the code would be better without them.
Making READ_COMMITTED_SNAPSHOT the default
As described in the related CodePlex issue, it seems that READ_COMMITTED_SNAPSHOT is generally a better default for new databases than the default for SQL Server, which is to use locks instead
of snapshots. This will often provide better scalability and fewer deadlocks. The proposal is to make this the default for all new databases created by Entity Framework—that is, when using Code First, Code First with Migrations, and Model First. We can change
the first two of these with a simple change in the runtime code.
The questions to be addressed before doing this are:
- Is it a better default?
- Is it a significant breaking change if we change the default for new databases.
Links in the CodePlex issue and discussions with other teams indicate that generally speaking it is a better default. This is further supported by the fact that it is the default for SQL Azure. There will always be cases where something else is better, but
this can be handled by altering the database, which can be done easily from a Migration or database initializer, or through a database management tool.
We came to the conclusion that it is not a significant breaking change for two reasons:
- We’re only doing this for new databases, so any existing application running on an existing database is not affected.
- It’s very hard to write applications using EF that depend on the different behavior between snapshots and locks. This is because:
- EF already implicitly uses snapshots because of caching in the state manager
- The change may mean that you get stale data instead of blocking and then getting fresh data, but unless you are going out of your way to prevent this it is already a race condition as to which you will get. You might now be more likely to get stale data,
but in most cases it was already possible and so your application would have been incorrect to assume that the data was not stale.
Therefore, we will make READ_COMMITTED_SNAPSHOT the default for new databases created by EF. We will document this: see
work item 688.
Database First WinForms data binding
We have guidance for doing Windows Forms data binding with DbContext and Code First. This document states that, “The walkthrough uses Code First to define the conceptual model, but you can
also use the Entity Framework Designer (EF Designer).” While this is true it is actually not very helpful because:
- The code generated by the DbContext templates does not match the code needed to do data binding and this is not called out. Specifically, the ObservableListSource class is not generated and navigation properties are not of the ObservableListSource type.
- There are no instructions on how to change the generated code to match that required for data binding, and it is not immediately obvious how to do this.
We have a few options here:
- Update the documentation to explain how to modify the generated code. The only reasonable way to do this is to modify the T4 templates. This is not difficult, but can be a bit scary and isn’t a great experience.
- Modify the DbContext templates and the designer so that the templates can generate the correct code when the user indicates in the designer that they are doing data binding. This will only help people who get the update to the designer (i.e. not people
using VS 2010, and probably not those using VS 2012) and it could also end up being not very discoverable.
- Publish new templates specifically for use when doing data binding. This is a decent idea, but there is significant overhead associated with supporting and maintaining templates which we want to avoid if possible.
- We will definitely update the documentation since that can be done now with little cost.
- We will consider creating new templates, but this may or may not fit in the schedule for EF6.
See CodePlex item 639.