-
Notifications
You must be signed in to change notification settings - Fork 230
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
Dot net standard #257
base: develop
Are you sure you want to change the base?
Dot net standard #257
Conversation
…gacy, Logging.Legacy
…gya/microdot into us/95878.infra.orleans.2_20.5
…throw meaningfull exception if failes)
@@ -321,8 +321,8 @@ private async Task SetResponse(HttpListenerContext context, string serviceName, | |||
|
|||
public void Dispose() | |||
{ | |||
_consulListener.Close(); | |||
((IDisposable)_consulListener)?.Dispose(); | |||
//_consulListener.Close(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should understand the issue.
dispose should not throw an exception or cause other side effects
@@ -2,7 +2,7 @@ | |||
<configuration> | |||
<Discovery> | |||
<Services> | |||
<CalculatorService Source="Local" /> | |||
<CalculatorService Source="Config" Hosts="127.0.0.1" DefaultPort="12323"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert
<TargetFramework>netstandard2.0</TargetFramework> | ||
<GenerateAssemblyInfo>false</GenerateAssemblyInfo> | ||
<GeneratePackageOnBuild>true</GeneratePackageOnBuild> | ||
<PackageVersion>7.0.3-beta</PackageVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The package version should be 6.
The build server Identify branch is not master and therefor create a beta version.
Also, we want one nuspec file that controls the version. ( microdot come as a full package)
@@ -69,13 +69,11 @@ public CurrentApplicationInfo(string name, string instanceName = null, Version i | |||
if (name == null) | |||
throw new ArgumentNullException(nameof(name)); | |||
|
|||
OsUser = WindowsIdentity.GetCurrent().Name; | |||
OsUser = "N/A"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you find a way in Linux to get the Current user?
// ReSharper disable once PossibleNullReferenceException | ||
IsRunningAsWindowsService = Environment.OSVersion.Platform == PlatformID.Win32NT && | ||
WindowsIdentity.GetCurrent().Name == @"NT AUTHORITY\SYSTEM"; | ||
IsRunningAsWindowsService = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@daniel-lamberger can we remove WindowsService support?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Environment.UserName
@@ -296,6 +297,10 @@ private async Task HandleRequest(HttpListenerContext context) | |||
} | |||
} | |||
} | |||
finally | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need more investigation to understand doker in Linux
@@ -71,7 +71,7 @@ public async Task<ConfigItemsCollection> GetConfiguration() | |||
.Select(match => new | |||
{ | |||
Placehodler = "%" + match.Groups[1].Value + "%", | |||
Value = _environmentVariableProvider.GetEnvironmentVariable(match.Groups[1].Value) | |||
Value = _environmentVariableProvider.GetEnvironmentVariable(match.Groups[1].Value.ToUpper()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Can you please move the ToUpper to the where.
- add readme that describes our environment variable format ( Upper case key, lower case value )
@@ -65,7 +65,28 @@ public ConfigurationFilesWatcher(IConfigurationLocationsParser configurationLoca | |||
|
|||
private void CreateRootWatcher() | |||
{ | |||
|
|||
var usePolling = Environment.GetEnvironmentVariable("GIGYA_CONFIG_USE_POLLING"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@daniel-lamberger what do you think about polling instead of Watcher.
(due to docker on windows have no support in file system Watcher)
@@ -97,7 +97,7 @@ public void OrleansSerialization_CustomerFacingException_IsEquivalent() | |||
/// [DONE] #ORLEANS20 - I don't now why, but round/trip for HttpRequestException is loosing stack trace... | |||
/// https://github.com/dotnet/orleans/issues/5876 | |||
/// </summary> | |||
[Test] | |||
[Test, Ignore("Waiting for issue to be fixed by Orleans")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's try to find other solution for that
@@ -356,7 +354,7 @@ public async Task RequestContextOverrideShouldFailOnFirstAttempt() | |||
|
|||
//Disable TracingContext.SetRequestID("1"); | |||
|
|||
CallContext.FreeNamedDataSlot("#ORL_RC"); | |||
TracingContext.ClearContext();// CallContext.FreeNamedDataSlot("#ORL_RC"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove zombi code :P
…n outgoing requests
No description provided.