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

Package migrations #1319

Open
wants to merge 9 commits into
base: dev-v7
from

Conversation

Projects
None yet
8 participants
@sitereactor
Copy link
Member

sitereactor commented Jun 13, 2016

Opening this PR to start a conversation about what it should contain / not contain before it can be merged into a future release.

  • Aggregated view of migrations to run (aggregate both Products and versions)
  • Holding page

Question: For package migrations should only the latest version be shown/saved to the migrations table or should all migrations that have run be shown?

Shazwazza and others added some commits Oct 2, 2015

adds another check to app context for HasPendingPackageMigrations, up…
…dates the installer auth checks and adds a new installer route which will be redirected to instead of the normal installer route when there are pending migrations.
Moves some logic to PackageMigrationsContext, ensures that the dest v…
…ersion for each package is based on it's maximum available version found in it's migrations.
Merge remote-tracking branch 'umbraco/dev-v7' into package-migrations
# Conflicts:
#	src/Umbraco.Web/Install/InstallHelper.cs
Minor tweaks to ExecutionMigrationStep and PackageMigrationsContext.
Adds tests for ExecutionMigrationStep and PackageMigrationsContext.
@Jeavon

This comment has been minimized.

Copy link
Contributor

Jeavon commented Jul 15, 2016

Any chance of sharing code for a test package that uses migrations that can be used for testing? Thanks!

@Shazwazza

This comment has been minimized.

Copy link
Member

Shazwazza commented Jul 15, 2016

Put some migrations in your package with your product name in the attribute and target the migration for a future version than is currently installed. Tada.

@sitereactor

This comment has been minimized.

Copy link
Member Author

sitereactor commented Jul 15, 2016

I have a solution with test migrations. If I'm near my laptop over the weekend I can drop you an email with the code.

@Jeavon

This comment has been minimized.

Copy link
Contributor

Jeavon commented Jul 15, 2016

Thanks @sitereactor that would be useful!

I've been experimenting a little and I have this code that works just fine:

using Umbraco.Core.Logging;
using Umbraco.Core.Persistence.Migrations;
using Umbraco.Core.Persistence.SqlSyntax;

namespace AwesomePackageIdea
{
    [Migration("2.0.0", 1, "AwesomePackageIdea")]

    public class Class1 : MigrationBase
    {
        public Class1(ISqlSyntaxProvider sqlSyntax, ILogger logger) : base(sqlSyntax, logger)
        {
        }

        public override void Up()
        {

        }

        public override void Down()
        {

        }
    }
}

Whilst this is totally awesome often packages need a way to get user input to complete configuration (e.g. a Azure storage connection string) is there/could there be provision to supply a path to a Angular view that is loaded once the migration has completed so that configuration can be dealt with by the package author (sort of like the old user control that is loaded after installing a current package)?

@Shazwazza

This comment has been minimized.

Copy link
Member

Shazwazza commented Jul 15, 2016

That hasn't been taken into consideration quite yet, I think it would be best to create a new feature request for this that relates to the original issue. We'll need to take into account how that will work when multiple packages are installed and also take into account that this process could be run silently so it won't be guaranteed that these views will get executed. I think we'll have to add this functionality to the new packaging system too, so devs can view the package's custom installer screen - which we still have the user control thing but in the future should be angular.

@Jeavon

This comment has been minimized.

Copy link
Contributor

Jeavon commented Jul 15, 2016

Good point about multiples and silent installation, might need a flag to specify if silent installation is supported/not. It would be cool if there was a way to view a packages installer view in the package UI, it would be up to the author what's showing there rather than creating a dashboard on install, e.g. change configuration etc...

I'll create a feature request attached to U4-8605

@Jeavon

This comment has been minimized.

@sitereactor

This comment has been minimized.

Copy link
Member Author

sitereactor commented Aug 5, 2016

@Jeavon Do you still want the migration test solution? Didn't get a chance to send it before my holidays.

@Jeavon

This comment has been minimized.

Copy link
Contributor

Jeavon commented Aug 5, 2016

@sitereactor no worries, I think I tested what I need to and created two new feature requests :)

@mortenbock

This comment has been minimized.

Copy link
Contributor

mortenbock commented Feb 13, 2017

@sitereactor @Shazwazza @Jeavon A thought about "post package install config".

Would it not be better if the "packages" tree would instead have a "config" tab for each package? That way you would always have a place to configure a package, and not necessarily need to do it right after install?

The package migrations screen could then just show a "make sure to configure the installed packages" message, maybe with a link to the packages that were just installed?

Each package could then build whatever they want in that config tab, using the same techniques used today when creating custom sections or other App_Plugins type extensions.

This would also create an alternative to packages installing dashboards, just to have a place where they can be configured?

@Shazwazza

This comment has been minimized.

Copy link
Member

Shazwazza commented Feb 13, 2017

@mortenbock It would be an interesting idea to allow a package manifest to declare a package 'options' view that can be configured in the packager area. However, package migrations when declared will need to be run on install because:

  • Just like Core migrations, some package migrations will be required to allow the site to run and without them in some cases could result in the entire site YSODing. This isn't true for every package but for many it might be
  • We run Core migrations in an 'install' state, which means during this state we don't boot all app event handlers among other things so that we can guarantee that the migrations run without a bunch of external influence. We would be running package migrations in the same app state

If we allowed a migration attribute to denote that it is not a critical migration and didn't force the migration install state to run, then having users required to manually apply the migrations comes with it's own challenges:

  • We would have to have global alerts in the back office since even though a package migration may not be critical, it will still probably mean that the package will not run correctly if not applied and people will forget to do this
  • We need to build up this migration runner toolbox and also ensure that these migrations run in an 'installer' state which means the CMS will not serve front-end requests during this time and will instead display the holding screen

Considering that, I'm not really sure there is much benefit of simply not running them on install just like we do when you upgrade Umbraco, IMO the experience should be consistent.

@mortenbock

This comment has been minimized.

Copy link
Contributor

mortenbock commented Feb 14, 2017

@Shazwazza I agree with you that "non mandatory" migrations do not make a lot of sense. I don't think i was suggesting that?

The "options" view, was just to have one canonical place to edit package specific stuff, such as API keys or AccountIds etc. So values, not schema changes.

Just seems like a good thing to be able to always go back and change it, without having to trigger some sort of "install" state.

@Shazwazza

This comment has been minimized.

Copy link
Member

Shazwazza commented Feb 15, 2017

@mortenbock I was referring to your comment: "not necessarily need to do it right after install?" which I am saying we cannot do because migrations are generally critical and need to be applied on startup during an install state. The only way that cannot happen is by having non-critical migrations which I've explained above is probably not a good idea.

I'm not sure what you mean by "Just seems like a good thing to be able to always go back and change it, without having to trigger some sort of 'install' state". As above, migrations will need to be run during startup in an install state, it will look similar to the installer screen and we will have an option for a holding page.

@nul800sebastiaan

This comment has been minimized.

Copy link
Member

nul800sebastiaan commented Feb 15, 2017

What Morten is talking about is simple thing like: you install a package that relies on Google Maps or the Twitter API, it needs an API key. Or a package allows you to optionally install some content after package install, see screenshot.

image

@nul800sebastiaan

This comment has been minimized.

Copy link
Member

nul800sebastiaan commented Feb 15, 2017

Or maybe some links to documentation 😉

image

@Shazwazza

This comment has been minimized.

Copy link
Member

Shazwazza commented Feb 15, 2017

Yup i fully understand that part and it would be totally great to have an 'options' view in the package manifest to show - which of course a package author could really have any functionality in there. We could even show the custom legacy user control thing in the package area for now too.

What I'm saying is that package migrations cannot be executed on these screens, they will be run at startup.

@mortenbock

This comment has been minimized.

Copy link
Contributor

mortenbock commented Feb 15, 2017

@Shazwazza Ah, I see the missing clarity in my original comment. I only meant that it would not be necessary to show the options view right after install.

Today, the legacy user control thing is displayed right after install, but in the description above, we might be installing/upgrading multiple packagers at once. So instead of showing 5 post-install/migration config screens, then just let the user go to each package to finish setting them up, after migrations have been run.

@Shazwazza

This comment has been minimized.

Copy link
Member

Shazwazza commented Feb 15, 2017

Yup! Think we're all on the same page :) sorry for the confusion !

@JimBobSquarePants

This comment has been minimized.

Copy link
Contributor

JimBobSquarePants commented Feb 21, 2017

Would it be possible to list the processing steps of a migration and what functionality is possible during each step?

What format you expect packages to be delivered via nuget. As an exported Zipfile?, A stream?

I ask because I have been experimenting with automatic installation of my package via files embedded in my binary.

https://github.com/JimBobSquarePants/Zoombraco/blob/2a4e974050a6c174d6d0dbd15f88a46ec2ef42e5/src/Zoombraco/ZoombracoBootstrapper.cs#L34

I can do most things except register the package as installed with the CMS due to the legacy code that controls that behaviour.

Ideally I would like to be able to install a single Zip file via a stream embedded embedded within my binary using migrations and I want to ensure anything designed covers a wide range of use cases.

@Shazwazza

This comment has been minimized.

Copy link
Member

Shazwazza commented Feb 22, 2017

@JimBobSquarePants Sounds like you think this might be something other than what it is. Here's what package Migrations are:

  • We already do this in Umbraco Core when an upgrade occurs, you can see examples here: https://github.com/umbraco/Umbraco-CMS/tree/dev-v7/src/Umbraco.Core/Persistence/Migrations/Upgrades
  • They are DB changes or even c# scripts that are executed when an upgrade is detected for the given version detected
  • We track all product migrations for a given version in the umbracoMigration table
  • When a migration for a product is run against a given target version, we add a row to the db
  • A migration is like other Umbraco plugins, we will either assembly scan for these classes or you'll have to manually register them on startup (haven't decided yet)

So this has nothing to do specifically with Nuget, zip files, streams, etc... it's just code.

Let's say you release a new minor version and you require some code or db changes to upgrade your package, you would create a migration class that targets this minor version. On startup, we would detect that there are outstanding migrations that haven't been executed for products that are required for the current version (as compared to with what we have tracked in the db)

@JimBobSquarePants

This comment has been minimized.

Copy link
Contributor

JimBobSquarePants commented Feb 22, 2017

Thanks for clarifying @Shazwazza I got here after following the ticket linked in our conversation on Twitter and reading the line.

With package migrations, we will be able to have packages become true Nuget packages that are installable via Visual Studio and also have the ability to install their data which will be done by launching the installer screen.

So this, in part, is a means to allow an installer screen from which we can then perform our own installation/uninstallation logic?

@Shazwazza

This comment has been minimized.

Copy link
Member

Shazwazza commented Feb 22, 2017

Yup so there's 2 parts to Nuget packages:

  • We need to have Package Migrations first (which is what this PR and task is about) so there's a single way in which a package can perform their own upgrade logic regardless of it's package type (i.e. Umbraco package format or Nuget package format). Currently there's the legacy 'Package Actions' but those don't work for Nuget packages and they also have their own flaws
  • We would like to unite the package formats (but Package Migrations need to come first). It should be relatively 'easy' to make the default Umbraco package format to actually be the Nuget package format. The caveats would be:
    • Obviously no powershell will be supported, so if that is in your Nuget package that will not execute if installed via the umbraco back office
    • Config transforms should be able to work if installed via the umbraco back office
    • If there is Umbraco data that needs to be installed that is currently supported by the Umbraco packager (i.e. things like content types and data types), then an umbraco package manifest file can be included in the Nuget package.
    • If the package is installed via the umbraco back office, this manifest would be automatically installed and then the packages migrations would be executed in the package installer UI
    • If the package is installed via Nuget/VS, then this manifest would be automatically installed during the package migration runner on site startup
@JimBobSquarePants

This comment has been minimized.

Copy link
Contributor

JimBobSquarePants commented Feb 22, 2017

Perfect! Thanks for clarifying. If you need my assistance in any way please let me know. 😄

@PeteDuncanson

This comment has been minimized.

Copy link
Contributor

PeteDuncanson commented Sep 12, 2018

Any chance of this one being bumped up the list as something to look at? Would be a sweeeeeet improvement

@stevetemple

This comment has been minimized.

Copy link
Contributor

stevetemple commented Nov 6, 2018

Other than resolving conflicts, reading through the commend history I can't see any things left to complete.

Is there is something I can do to help get this shuffled along?

@Shazwazza

This comment has been minimized.

Copy link
Member

Shazwazza commented Nov 6, 2018

It's been years since I've looked at this and tested it so can't remember off the top of my head.

There's a few points we need to do and decide on:

  • We want to be able to have a configuration option for a holding page (noted in the title comment of this PR). If a holding page is configured and upgrades need to be run for either Umbraco or for a Package, the holding page will be shown ... this must only be an html page.
  • Aggregated view of migrations to run (aggregate both Products and versions) (noted in the title comment of this PR). - I think this meant to add the version to this page migrations.html
  • Question: For package migrations should only the latest version be shown/saved to the migrations table or should all migrations that have run be shown? (noted in the title comment of this PR). IMO - we should follow the same pattern we follow for the CMS. (in v8 however, this is already different)
  • If the package is installed/upgraded via the back office, the installer will run the migrations during the installation process
  • The current implementation will show the installer screen and prevent Umbraco from running/booting until package migrations are executed. This is because we don't know if the site would actually be able to run without the packages migrations being executed. In many cases this will be true, however in some cases this might not be true - so would we want to be able to decorate migrations when a package developer knows that the migration is not necessary for the site to run? In which case, the site owner could run the migrations inside the back office and it wouldn't prevent the site from starting. ... This could also be a "v2" feature of Package Migrations so could wait.

v8 migrations are quite different from this concept so will need a bit of work to merge these changes upward in v8.

This PR was briefly discussed early this year and we realized there is a challenge with this which was something to do with deployments between environments and things like Umbraco Cloud. I guess if you were to deploy changes between environments, then of course the migration screen will show because the migrations haven't been run on that environment/database... that is expected/wanted. I suppose Umbraco Cloud would need to be updated to also support package migrations so it might be able to magically run them behind the scenes when transferring from one environment to another (like it does for upgrade changes).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.