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

Port config fix to 2.2 #1221

Merged
merged 8 commits into from
Mar 15, 2019
Merged

Port config fix to 2.2 #1221

merged 8 commits into from
Mar 15, 2019

Conversation

HaoK
Copy link
Member

@HaoK HaoK commented Mar 5, 2019

@HaoK
Copy link
Member Author

HaoK commented Mar 6, 2019

There's no reload for user secrets in 2.2 so those tests had to be removed

@ajcvickers ajcvickers added this to the 2.2.x milestone Mar 7, 2019
@ajcvickers ajcvickers added the servicing-consider Shiproom approval is required for the issue label Mar 7, 2019
@ajcvickers
Copy link
Member

Original PR: #1202
Issue: #1189

Description

There is a race condition that can be hit if a file is being updated while the configuration system is refreshing configuration from that file.

Customer Impact

Customer reported. Application crashes with exception.

Regression?

No; same behavior in 2.1. At this time, we're bring for 2.2 only.

Risk

Low. Configuration system is relatively small and code change is simple.

@martincostello
Copy link
Member

For my own curiosity, how often does "Shiproom" meet to discuss such proposed changes?

@ajcvickers
Copy link
Member

@martincostello Usually twice per week.

@martincostello
Copy link
Member

Thanks - I was just wondering as I was reviewing our production logs from over the weekend and we had a handful of the exception occur, so I just wanted to what the status was.

@dougbu
Copy link
Member

dougbu commented Mar 14, 2019

This fix is incomplete without an update to eng/PatchConfig.props.

@vivmishra vivmishra added servicing-approved and removed servicing-consider Shiproom approval is required for the issue labels Mar 14, 2019
@vivmishra vivmishra modified the milestones: 2.2.x, 2.2.4 Mar 14, 2019
@vivmishra
Copy link

Approved for 2.2.4
Checkin after branding update @dougbu

@HaoK
Copy link
Member Author

HaoK commented Mar 14, 2019

@dougbu what am I supposed to do to patchconfig.props?

@martincostello
Copy link
Member

I think Microsoft.Extensions.Configuration.KeyPerFile and Microsoft.Extensions.Configuration.EnvironmentVariables need adding to the 2.2.4 node.

@HaoK
Copy link
Member Author

HaoK commented Mar 14, 2019

Alright, @dougbu updated patchconfig

Copy link
Member

@dougbu dougbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks complete from an eng perspective.

@HaoK I resolved the conflict with my branding update. You're good to go here because that's in. (2.1.10 branding is still pending right this minute.)

@HaoK
Copy link
Member Author

HaoK commented Mar 14, 2019

Hrm build checks are red after updating the patch config... any ideas @dougbu ?

WARNING: 0 error(s) and 2 warning(s) found with package Microsoft.Extensions.Configuration.EnvironmentVariables (2.2.4-servicing-19164-12)
WARNING: ASSEMBLY_FILE_VERSION_MISMATCH @ lib/netstandard2.0/Microsoft.Extensions.Configuration.EnvironmentVariables.dll: The managed assembly file version '2.2.0.19073' does not match the package version '2.2.4-servicing-19164-12' for package Microsoft.Extensions.Configuration.EnvironmentVariables.
WARNING: ASSEMBLY_VERSION_MISMATCH @ lib/netstandard2.0/Microsoft.Extensions.Configuration.EnvironmentVariables.dll: The managed assembly version '2.2.0.0' does not match the package version '2.2.4.0' for package Microsoft.Extensions.Configuration.EnvironmentVariables.
INFO: Analyzing Microsoft.Extensions.Configuration.KeyPerFile (2.2.4-servicing-19164-12)
WARNING: 0 error(s) and 2 warning(s) found with package Microsoft.Extensions.Configuration.KeyPerFile (2.2.4-servicing-19164-12)
WARNING: ASSEMBLY_FILE_VERSION_MISMATCH @ lib/netstandard2.0/Microsoft.Extensions.Configuration.KeyPerFile.dll: The managed assembly file version '2.2.0.19073' does not match the package version '2.2.4-servicing-19164-12' for package Microsoft.Extensions.Configuration.KeyPerFile.
WARNING: ASSEMBLY_VERSION_MISMATCH @ lib/netstandard2.0/Microsoft.Extensions.Configuration.KeyPerFile.dll: The managed assembly version '2.2.0.0' does not match the package version '2.2.4.0' for package Microsoft.Extensions.Configuration.KeyPerFile.
WARNING: Unnecessary exclusion in Microsoft.Extensions.Logging.Testing
Issue: BUILD_ITEMS_FRAMEWORK
Instance: *

Showing JSON for ignore content:
{
dotnet.exe failed with exit code: 1
  "Microsoft.Extensions.Configuration.EnvironmentVariables": {
At D:\a\.dotnet\buildtools\korebuild\2.2.1-build-20190104.5\scripts\common.psm1:11 char:9
+         throw "$cmdName failed with exit code: $exitCode"
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : OperationStopped: (dotnet.exe failed with exit code: 1:String) [], RuntimeException
    + FullyQualifiedErrorId : dotnet.exe failed with exit code: 1
 
    "Exclusions": {
      "ASSEMBLY_FILE_VERSION_MISMATCH": {
        "lib/netstandard2.0/Microsoft.Extensions.Configuration.EnvironmentVariables.dll": "Enter justification"
      },
      "ASSEMBLY_VERSION_MISMATCH": {
        "lib/netstandard2.0/Microsoft.Extensions.Configuration.EnvironmentVariables.dll": "Enter justification"
      }
    }
  },
  "Microsoft.Extensions.Configuration.KeyPerFile": {
    "Exclusions": {
      "ASSEMBLY_FILE_VERSION_MISMATCH": {
        "lib/netstandard2.0/Microsoft.Extensions.Configuration.KeyPerFile.dll": "Enter justification"
      },
      "ASSEMBLY_VERSION_MISMATCH": {
        "lib/netstandard2.0/Microsoft.Extensions.Configuration.KeyPerFile.dll": "Enter justification"
      }
    }
  }
}

@dougbu
Copy link
Member

dougbu commented Mar 14, 2019

Oh well, this isn't quite ready to go

@dougbu
Copy link
Member

dougbu commented Mar 14, 2019

@HaoK will get back to you on the version mismatches once I get home. Things aren't working as they should…

@dougbu
Copy link
Member

dougbu commented Mar 15, 2019

@HaoK didn't find similar changes elsewhere,. But, I'm guessing the package verifier is correct.

Add the following to the Default/packages array in eng/NuGetPackageVerifier.json:

  "Microsoft.Extensions.Configuration.EnvironmentVariables": {
    "Exclusions": {
      "ASSEMBLY_FILE_VERSION_MISMATCH": {
        "lib/netstandard2.0/Microsoft.Extensions.Configuration.EnvironmentVariables.dll": "Enter justification"
      },
      "ASSEMBLY_VERSION_MISMATCH": {
        "lib/netstandard2.0/Microsoft.Extensions.Configuration.EnvironmentVariables.dll": "Enter justification"
      }
    }
  },
  "Microsoft.Extensions.Configuration.KeyPerFile": {
    "Exclusions": {
      "ASSEMBLY_FILE_VERSION_MISMATCH": {
        "lib/netstandard2.0/Microsoft.Extensions.Configuration.KeyPerFile.dll": "Enter justification"
      },
      "ASSEMBLY_VERSION_MISMATCH": {
        "lib/netstandard2.0/Microsoft.Extensions.Configuration.KeyPerFile.dll": "Enter justification"
      }
    }
  }

@HaoK
Copy link
Member Author

HaoK commented Mar 15, 2019

Cool that was it, green now @dougbu

@dougbu dougbu merged commit 9ebff1a into release/2.2 Mar 15, 2019
@dougbu dougbu deleted the haok-port branch March 15, 2019 04:14
JunTaoLuo pushed a commit to dotnet/aspnetcore that referenced this pull request Feb 12, 2020
- port of dotnet/extensions#1202
- with PR tweaks for 2.2
  - e.g. adjust Microsoft.Extensions.Configuration.FunctionalTests.csproj to match layout here
- update PatchConfig.props and NuGetPackageVerifier.json\n\nCommit migrated from dotnet/extensions@9ebff1a
JunTaoLuo pushed a commit to dotnet/aspnetcore that referenced this pull request Feb 15, 2020
- port of dotnet/extensions#1202
- with PR tweaks for 2.2
  - e.g. adjust Microsoft.Extensions.Configuration.FunctionalTests.csproj to match layout here
- update PatchConfig.props and NuGetPackageVerifier.json\n\nCommit migrated from dotnet/extensions@9ebff1a
maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Feb 28, 2020
- port of dotnet/extensions#1202
- with PR tweaks for 2.2
  - e.g. adjust Microsoft.Extensions.Configuration.FunctionalTests.csproj to match layout here
- update PatchConfig.props and NuGetPackageVerifier.json


Commit migrated from dotnet/extensions@9ebff1a
maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Mar 2, 2020
- port of dotnet/extensions#1202
- with PR tweaks for 2.2
  - e.g. adjust Microsoft.Extensions.Configuration.FunctionalTests.csproj to match layout here
- update PatchConfig.props and NuGetPackageVerifier.json


Commit migrated from dotnet/extensions@9ebff1a
maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Mar 11, 2020
- port of dotnet/extensions#1202
- with PR tweaks for 2.2
  - e.g. adjust Microsoft.Extensions.Configuration.FunctionalTests.csproj to match layout here
- update PatchConfig.props and NuGetPackageVerifier.json


Commit migrated from dotnet/extensions@9ebff1a
maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Mar 27, 2020
- port of dotnet/extensions#1202
- with PR tweaks for 2.2
  - e.g. adjust Microsoft.Extensions.Configuration.FunctionalTests.csproj to match layout here
- update PatchConfig.props and NuGetPackageVerifier.json


Commit migrated from dotnet/extensions@9ebff1a
@ghost ghost locked as resolved and limited conversation to collaborators May 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants