Skip to content
This repository has been archived by the owner on Jul 30, 2024. It is now read-only.
/ NuGet.Jobs Public archive

Update jobs to latest SQL AAD #485

Merged
merged 9 commits into from
Jul 24, 2018
Merged

Update jobs to latest SQL AAD #485

merged 9 commits into from
Jul 24, 2018

Conversation

chenriksson
Copy link
Member

@chenriksson chenriksson commented Jul 19, 2018

  • Updates to NuGet.Services.Sql version with token cache fixes (Replace default ADAL TokenCache ServerCommon#172)
  • Adds logging around token acquisition
  • Refactoring to reduce NuGet.Services.Sql dependency to just Jobs.Common project for easier maintenance
  • Added connection test at startup for better testing and diagnostics

/// Used for diagnostics when a connection isn't yet established.
/// Consider removing once ConnectionStringBuilder property is exposed on AzureSqlConnectionFactory.
/// </summary>
public class DatabaseIdentifier
Copy link
Member Author

Choose a reason for hiding this comment

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

Will remove once I expose SqlConnectionStringBuilder property on factory.

@@ -82,6 +82,9 @@
<PackageReference Include="NuGet.Services.Logging">
<Version>2.25.0</Version>
</PackageReference>
<PackageReference Include="NuGet.Services.Sql">
<Version>2.26.0-chenriks-aadsql-35160</Version>
Copy link
Member Author

Choose a reason for hiding this comment

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

Will update to master build before merge

@@ -75,5 +75,8 @@
<Version>2.3.1</Version>
</PackageReference>
</ItemGroup>
<ItemGroup>
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
</ItemGroup>
Copy link
Member Author

@chenriksson chenriksson Jul 19, 2018

Choose a reason for hiding this comment

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

Caused by microsoft/vstest#472

Will keep as-is since other test projects already have this. We can clean up across projects/repos once everyone is using 15.7.

@@ -80,5 +80,8 @@
<Name>Tests.ContextHelpers</Name>
</ProjectReference>
</ItemGroup>
<ItemGroup>
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
</ItemGroup>
Copy link
Member Author

@chenriksson chenriksson Jul 19, 2018

Choose a reason for hiding this comment

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

Caused by microsoft/vstest#472

Will keep as-is since other test projects already have this. We can clean up across projects/repos once everyone is using 15.7.

/// Initializes an <see cref="ISqlConnectionFactory"/>, for use by non-validation jobs.
/// </summary>
/// <returns>ConnectionStringBuilder, used for diagnostics.</returns>
public SqlConnectionStringBuilder RegisterDatabase(IServiceContainer serviceContainer, IDictionary<string, string> jobArgsDictionary, string argName)
Copy link
Member

Choose a reason for hiding this comment

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

argName -> connectionStringArgName?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

/// <summary>
/// Creates and opens a SqlConnection, for use by non-validation jobs.
/// </summary>
public Task<SqlConnection> OpenSqlConnectionAsync(string argName)
Copy link
Contributor

Choose a reason for hiding this comment

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

argName [](start = 65, length = 7)

What do you think of connectionName instead of argName?

Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed connectionStringArgName

@@ -159,13 +164,10 @@ private void ConfigureLibraries(IServiceCollection services)
services.AddLogging();
}

private DbConnection CreateDbConnection<T>(IServiceProvider serviceProvider) where T : IDbConfiguration
private DbConnection CreateSqlConnection<T>()
Copy link
Contributor

Choose a reason for hiding this comment

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

CreateSqlConnection [](start = 29, length = 19)

This change also needs to be done in the JsonConfigurationJob

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved synchronous API to JobBase

@chenriksson
Copy link
Member Author

@joelverhagen @loic-sharma Updated with your feedback - thanks!

@chenriksson chenriksson changed the base branch from master to dev July 24, 2018 17:13
@chenriksson chenriksson merged commit d4e769d into dev Jul 24, 2018
@chenriksson chenriksson deleted the chenriks-aadsql branch July 24, 2018 17:21
chenriksson added a commit that referenced this pull request Jul 25, 2018
chenriksson added a commit that referenced this pull request Jul 25, 2018
chenriksson added a commit that referenced this pull request Jul 25, 2018
joelverhagen pushed a commit that referenced this pull request Sep 27, 2019
Adds the following custom analyzers:

1. The `ExactMatchCustomAnalyzer` that allows for case insensitive exact matches. This is used for the `packageId`, `version`, `owner`, and `owners` query fields.
2. The `PackageIdCustomAnalyzer` that splits on non alpha-numeric characters and camel casing. This is used by the `id` query field. This corresponds to the existing [`IdentifierAnalyzer`](https://github.com/NuGet/NuGet.Services.Metadata/blob/master/src/NuGet.Indexing/IdentifierAnalyzer.cs)
3. The `DescriptionCustomAnalyzer` that splits on non alpha-numeric characters and camel casing and removes stopwords (like "the" or "and"). This is used by the `title`, `description`, `author`, `authors`, and `summary` query fields. This corresponds to the existing [`DescriptionAnalyzer`](https://github.com/NuGet/NuGet.Services.Metadata/blob/master/src/NuGet.Indexing/DescriptionAnalyzer.cs)

Addresses NuGet/NuGetGallery#6920 and NuGet/NuGetGallery#6922

Also, see functional tests: NuGet/NuGet.Services.Metadata#487
joelverhagen pushed a commit that referenced this pull request Oct 26, 2020
joelverhagen pushed a commit that referenced this pull request Oct 26, 2020
joelverhagen pushed a commit that referenced this pull request Oct 26, 2020
joelverhagen pushed a commit that referenced this pull request Oct 26, 2020
Adds the following custom analyzers:

1. The `ExactMatchCustomAnalyzer` that allows for case insensitive exact matches. This is used for the `packageId`, `version`, `owner`, and `owners` query fields.
2. The `PackageIdCustomAnalyzer` that splits on non alpha-numeric characters and camel casing. This is used by the `id` query field. This corresponds to the existing [`IdentifierAnalyzer`](https://github.com/NuGet/NuGet.Services.Metadata/blob/master/src/NuGet.Indexing/IdentifierAnalyzer.cs)
3. The `DescriptionCustomAnalyzer` that splits on non alpha-numeric characters and camel casing and removes stopwords (like "the" or "and"). This is used by the `title`, `description`, `author`, `authors`, and `summary` query fields. This corresponds to the existing [`DescriptionAnalyzer`](https://github.com/NuGet/NuGet.Services.Metadata/blob/master/src/NuGet.Indexing/DescriptionAnalyzer.cs)

Addresses NuGet/NuGetGallery#6920 and NuGet/NuGetGallery#6922

Also, see functional tests: NuGet/NuGet.Services.Metadata#487
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants