Migrating data to newUserFunctionId tables as part of Consumption plan support #1071
Conversation
MaddyDev
requested review from
Charles-Gagnon,
lucyzhang929,
chlafreniere and
VasuBhog
as code owners
| @@ -54,6 +54,15 @@ public static string GetConnectionString(string connectionStringSetting, IConfig | |||
| return connectionString; | |||
| } | |||
|
|
|||
| public static string GetWebSiteName(IConfiguration configuration) | |||
| { | |||
| if (configuration == null) | |||
There was a problem hiding this comment.
What happens for local development? Is website site name set?
There was a problem hiding this comment.
WEBSITE_SITE_NAME value is null for local development, it creates the hex value for userfunctionId only using the functionName for local development
| @@ -78,8 +78,9 @@ public async Task<IListener> CreateListenerAsync(ListenerFactoryContext context) | |||
| { | |||
| _ = context ?? throw new ArgumentNullException(nameof(context), "Missing listener context"); | |||
|
|
|||
| string userFunctionId = await this.GetUserFunctionIdAsync(); | |||
| return new SqlTriggerListener<T>(this._connectionString, this._tableName, this._leasesTableName, userFunctionId, context.Executor, this._sqlOptions, this._logger, this._configuration); | |||
| string userFunctionId = this.GetUserFunctionId(); | |||
There was a problem hiding this comment.
Even after a migration is done once, are we still calculating these old + new values? Is that really needed?
There was a problem hiding this comment.
Yes, we decided to have the queries such that if old values are present, migrate them and clean up else continue with new values.
| BEGIN | ||
| -- Migrate LastSyncVersion from oldUserFunctionId if it exists and delete the record | ||
| Declare @lastSyncVersion bigint; | ||
| Select @lastSyncVersion = LastSyncVersion from az_func.GlobalState where UserFunctionID = '{this._oldUserFunctionId}' AND UserTableID = {userTableId} |
There was a problem hiding this comment.
Plz check for sql injection possibilities
|
@Charles-Gagnon can you take another look? |
There was a problem hiding this comment.
I would definitely recommend updating the README/MS docs to make it clear how the app name relates to the table and the consequences of changing that.
One thing I'm not sure we tested before is whether changing the app name previously would cause the host ID to change (and thus "lose" the history). I believe that would have been the case even previously, but now it's much more explicit so it'd be good to add that information in.
Will be adding the docs along with an end to end test in a separate PR. Thanks for calling this out. |
New UserFunctionId:
We have used the WEBSITE_SITE_NAME instead of the host id for creating the new userFunctionId
Migration steps followed:
Test: To Be Done