This repository has been archived by the owner on Jul 30, 2024. It is now read-only.
Status aggregator job #494
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| // Copyright (c) .NET Foundation. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| using Microsoft.Extensions.Logging; | ||
| using System; | ||
|
|
||
| namespace NuGet.Jobs.Extensions | ||
| { | ||
| public static class LoggerExtensions | ||
|
There was a problem hiding this comment. Please add comments. Why the current login is not enough? Why this method should be used over other methods? There was a problem hiding this comment.
Will add comments. |
||
| { | ||
| public static IDisposable Scope( | ||
| this ILogger logger, | ||
| string message, | ||
| params object[] args) | ||
| { | ||
| return new LoggerScopeHelper(logger, message, args); | ||
| } | ||
|
|
||
| private class LoggerScopeHelper : IDisposable | ||
| { | ||
| private readonly ILogger _logger; | ||
| private readonly IDisposable _scope; | ||
|
|
||
| private readonly string _message; | ||
| private readonly object[] _args; | ||
|
|
||
| private bool _isDisposed = false; | ||
|
|
||
| public LoggerScopeHelper( | ||
| ILogger logger, string message, object[] args) | ||
| { | ||
| _logger = logger; | ||
| _message = message; | ||
| _args = args; | ||
|
|
||
| _scope = logger.BeginScope(_message, _args); | ||
| _logger.LogInformation("Entering scope: " + _message, _args); | ||
| } | ||
|
|
||
| public void Dispose() | ||
| { | ||
| if (!_isDisposed) | ||
| { | ||
| _logger.LogInformation("Leaving scope: " + _message, _args); | ||
| _scope?.Dispose(); // ILogger can return a null scope (most notably during testing with a Mock<ILogger>) | ||
| _isDisposed = true; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| using System; | ||
|
|
||
| namespace NuGet.Jobs.Extensions | ||
| { | ||
| public static class ServiceProviderExtensions | ||
| { | ||
| public static T GetService<T>(this IServiceProvider provider) | ||
|
There was a problem hiding this comment.
This already exists: There was a problem hiding this comment. Removed and replaced the uses of this new extension |
||
| { | ||
| return (T)provider.GetService(typeof(T)); | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| // Copyright (c) .NET Foundation. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| using NuGet.Services.Status; | ||
|
|
||
| namespace StatusAggregator | ||
| { | ||
| public static class ComponentFactory | ||
|
There was a problem hiding this comment. Please add comments. What is a Component? There was a problem hiding this comment. NuGet/ServerCommon#170 for more context. I agree that this class should have more comments however. |
||
| { | ||
| public const string RootName = "NuGet"; | ||
| public const string GalleryName = "NuGet.org"; | ||
| public const string RestoreName = "Restore"; | ||
| public const string SearchName = "Search"; | ||
| public const string UploadName = "Package Publishing"; | ||
|
|
||
| public const string V2ProtocolName = "V2 Protocol"; | ||
| public const string V3ProtocolName = "V3 Protocol"; | ||
|
|
||
| public const string GlobalRegionName = "Global"; | ||
| public const string ChinaRegionName = "China"; | ||
|
|
||
| public const string UsncInstanceName = "North Central US"; | ||
| public const string UsscInstanceName = "South Central US"; | ||
| public const string EaInstanceName = "East Asia"; | ||
| public const string SeaInstanceName = "Southeast Asia"; | ||
|
|
||
| /// <summary> | ||
| /// Creates the NuGet service root component. | ||
| /// </summary> | ||
| public static IComponent CreateNuGetServiceRootComponent() | ||
| { | ||
| return new TreeComponent( | ||
| RootName, | ||
| "", | ||
| new IComponent[] | ||
| { | ||
| new PrimarySecondaryComponent( | ||
| GalleryName, | ||
| "Browsing the Gallery website", | ||
| new[] | ||
| { | ||
| new LeafComponent(UsncInstanceName, "Primary region"), | ||
| new LeafComponent(UsscInstanceName, "Backup region") | ||
| }), | ||
| new TreeComponent( | ||
| RestoreName, | ||
| "Downloading and installing packages from NuGet", | ||
| new IComponent[] | ||
| { | ||
| new TreeComponent( | ||
| V3ProtocolName, | ||
| "Restore using the V3 API", | ||
| new[] | ||
| { | ||
| new LeafComponent(GlobalRegionName, "V3 restore for users outside of China"), | ||
| new LeafComponent(ChinaRegionName, "V3 restore for users inside China") | ||
| }), | ||
| new PrimarySecondaryComponent( | ||
| V2ProtocolName, | ||
| "Restore using the V2 API", | ||
| new[] | ||
| { | ||
| new LeafComponent(UsncInstanceName, "Primary region"), | ||
| new LeafComponent(UsscInstanceName, "Backup region") | ||
| }) | ||
| }), | ||
| new TreeComponent( | ||
| SearchName, | ||
| "Searching for new and existing packages in Visual Studio or the Gallery website", | ||
| new[] | ||
| { | ||
| new PrimarySecondaryComponent( | ||
| GlobalRegionName, | ||
| "Search for packages outside China", | ||
| new[] | ||
| { | ||
| new LeafComponent(UsncInstanceName, "Primary region"), | ||
| new LeafComponent(UsscInstanceName, "Backup region") | ||
| }), | ||
| new PrimarySecondaryComponent( | ||
| ChinaRegionName, | ||
| "Search for packages inside China", | ||
| new[] | ||
| { | ||
| new LeafComponent(EaInstanceName, "Primary region"), | ||
| new LeafComponent(SeaInstanceName, "Backup region") | ||
| }) | ||
| }), | ||
| new LeafComponent(UploadName, "Uploading new packages to NuGet.org") | ||
| }); | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| // Copyright (c) .NET Foundation. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| using System; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.Extensions.Logging; | ||
| using NuGet.Jobs.Extensions; | ||
| using NuGet.Services.Status.Table; | ||
| using StatusAggregator.Table; | ||
|
|
||
|
|
||
| namespace StatusAggregator | ||
|
There was a problem hiding this comment.
There's two empty lines above this one |
||
| { | ||
| public class Cursor : ICursor | ||
| { | ||
| public Cursor( | ||
| ITableWrapper table, | ||
| ILogger<Cursor> logger) | ||
| { | ||
| _table = table; | ||
|
There was a problem hiding this comment.
I would add null checks to all constructors. From my experience, it makes catching null reference exceptions easier :) |
||
| _logger = logger; | ||
| } | ||
|
|
||
| private readonly ITableWrapper _table; | ||
|
|
||
| private readonly ILogger<Cursor> _logger; | ||
|
|
||
| public async Task<DateTime> Get() | ||
| { | ||
| using (_logger.Scope("Fetching cursor.")) | ||
| { | ||
| var cursor = await _table.Retrieve<CursorEntity>( | ||
| CursorEntity.DefaultPartitionKey, CursorEntity.DefaultRowKey); | ||
|
There was a problem hiding this comment. Is it ok to get a null cursor back? Should it throw instead of setting to minvalue? There was a problem hiding this comment. If there's no cursor, it likely means the table is empty and this is the first time the job has been run. So it should start at the beginning of time. There was a problem hiding this comment. I'll add a comment to make this behavior clearer. |
||
|
|
||
| DateTime value; | ||
| if (cursor == null) | ||
| { | ||
| value = DateTime.MinValue; | ||
| _logger.LogInformation("Could not fetch cursor."); | ||
| } | ||
|
There was a problem hiding this comment. It's not super clear from this log that the cursor will be set to the beginning of time. Maybe log the value here? Feel free to ignore There was a problem hiding this comment. I'll add that to the log |
||
| else | ||
| { | ||
| value = cursor.Value; | ||
| _logger.LogInformation("Fetched cursor with value {Cursor}.", value); | ||
| } | ||
|
|
||
| return value; | ||
| } | ||
| } | ||
|
|
||
| public Task Set(DateTime value) | ||
| { | ||
| using (_logger.Scope("Updating cursor to {Cursor}.", value)) | ||
| { | ||
| var cursorEntity = new CursorEntity(value); | ||
| return _table.InsertOrReplaceAsync(cursorEntity); | ||
|
There was a problem hiding this comment. Is this table thread safe? Does it need to be thread safe? There was a problem hiding this comment. Please add comments to this either way. There was a problem hiding this comment. It's Azure table storage, which is thread safe. |
||
| } | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Is this method used anywhere?
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.
I added it initially, must have removed it. Great catch!