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

Migration name filter #23222

Closed
lugospod opened this issue Nov 6, 2020 · 9 comments · Fixed by #26396
Closed

Migration name filter #23222

lugospod opened this issue Nov 6, 2020 · 9 comments · Fixed by #26396
Labels
area-migrations closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported good first issue This issue should be relatively straightforward to fix. type-enhancement
Milestone

Comments

@lugospod
Copy link

lugospod commented Nov 6, 2020

EF Core uses a class Migration, and if you add a new migration with name "Migration" it will break the build.

dotnet ef migrations add Migration

results with Circular base class dependency involving 'Migration' and 'Migration'

I recommend that "migrations add" adds a validation for this...

Include stack traces

Migrations/20201106105308_Migration.cs(6,26): error CS0146: Circular base class dependency involving 'Migration' and 'Migration' [/opt/app/src/src/NM.DAL/NM.DAL.csproj]
Migrations/20201106105308_Migration.cs(8,33): error CS0115: 'Migration.Up(MigrationBuilder)': no suitable method found to override [/opt/app/src/src/NM.DAL/NM.DAL.csproj]
Migrations/20201106105308_Migration.cs(167,33): error CS0115: 'Migration.Down(MigrationBuilder)': no suitable method found to override [/opt/app/src/src/NM.DAL/NM.DAL.csproj]
Migrations/20201106105308_Migration.Designer.cs(16,33): error CS0115: 'Migration.BuildTargetModel(ModelBuilder)': no suitable method found to override [/opt/app/src/src/NM.DAL/NM.DAL.csproj]

Include provider and version information

EF Core version: 3.1.9
Database provider: Postgresql
Target framework: NET 3.1
Operating system: Windows 10
IDE: Visual Studio 2019 16.7.7

@ajcvickers
Copy link
Member

Note from triage: we should add this and other similar conflicts to the check we already have. But low priority.

@ajcvickers ajcvickers added type-enhancement good first issue This issue should be relatively straightforward to fix. area-migrations labels Nov 7, 2020
@ajcvickers ajcvickers added this to the Backlog milestone Nov 7, 2020
@wreuel
Copy link

wreuel commented Mar 28, 2021

How could I help with that?

@ZephArvanitis
Copy link

Would this line/file be the right place to add this check?

protected override void Validate()
{
base.Validate();
if (string.IsNullOrEmpty(_name!.Value))
{
throw new CommandException(Resources.MissingArgument(_name.Name));
}
}

@KevRitchie
Copy link
Contributor

Hi @ajcvickers, is this still up for grabs?

@ajcvickers
Copy link
Member

@KevRitchie Yes.

@KevRitchie
Copy link
Contributor

Thanks @ajcvickers.

In summary, the changes I'm proposing are:

  • Add a check during the Validate method of MigrationsAddCommand to throw a CommandException when someone tries to call their migration "migration".
  • Add a new resource "CircularBaseClassDependency" to notify the user that "You cannot add a migration with the name 'Migration'".

Does that look OK?

I'm also trying to track down the tests for the MigrationsAddCommand, but can't find any. Could you point me in the right direction?

Thanks 😄

@ajcvickers
Copy link
Member

@bricelam Can you comment on the proposed design here?

@KevRitchie
Copy link
Contributor

@ajcvickers @bricelam - I have a version ready to go 😄 Is there anything else you think I need to add/consider (based on the above implementation)?

@bricelam
Copy link
Contributor

LGTM Feel free to send a PR

AndriySvyryd pushed a commit that referenced this issue May 19, 2022
Add a check during the Validate method of MigrationsAddCommand to throw a CommandException when someone tries to call their migration "migration".

Fixes #23222
@AndriySvyryd AndriySvyryd modified the milestones: Backlog, 7.0.0 May 19, 2022
@AndriySvyryd AndriySvyryd added community-contribution closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. labels May 19, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0, 7.0.0-preview5 May 25, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-preview5, 7.0.0 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migrations closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported good first issue This issue should be relatively straightforward to fix. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants