Add OracleManagedDataAccess Aspire Component by andrevlins · Pull Request #1004 · microsoft/aspire

andrevlins · 2023-11-23T02:24:39Z

Adds the ability to connect to an Oracle database through the OracleManagedDataAccess library by adding a connection of type OracleConnection or DbConnection.

It also adds the ability to configure a container with an Oracle Database Free image (container-registry.oracle.com/database/free:latest).

To continue this work I need the following packages to be available in the nuget repository of this project:

AspNetCore.HealthChecks.Oracle 7.0.0
Oracle.ManagedDataAccess.Core 23.3.0-dev
Oracle.ManagedDataAccess.OpenTelemetry 23.2.0-dev

andrevlins · 2023-11-23T13:50:31Z

Hello @RussKie, Can you help me with these packages mentioned above?

AspNetCore.HealthChecks.Oracle 7.0.0
Oracle.ManagedDataAccess.Core 23.3.0-dev
Oracle.ManagedDataAccess.OpenTelemetry 23.2.0-dev

davidfowl · 2023-11-24T08:44:01Z

src/Components/Aspire.OracleManagedDataAccess/AspireOracleManagedDataAccessExtensions.cs

+            {
+                ValidateConnection();
+
+                return new OracleConnection(settings.ConnectionString);


Oracle doesn't have a data source?

No. I intend to update this code as soon as they implement DbDataSource.

FYI

oracle/dotnet-db-samples#329

mitchdenny · 2023-11-27T22:42:41Z

src/Components/Aspire.OracleManagedDataAccess/AspireOracleManagedDataAccessExtensions.cs

+    /// <param name="builder">The <see cref="IHostApplicationBuilder" /> to read config from and add services to.</param>
+    /// <param name="connectionName">A name used to retrieve the connection string from the ConnectionStrings configuration section.</param>
+    /// <param name="configureSettings">An optional delegate that can be used for customizing options. It's invoked after the settings are read from the configuration.</param>
+    /// <remarks>Reads the configuration from "Aspire:Oracle" section.</remarks>


Aspire:OracleManagedDataAccess

Changed to Aspire:Oracle:ManagedDataAccess:Core as @eerhardt advised

mitchdenny · 2023-11-27T23:54:48Z

tests/Aspire.OracleManagedDataAccess.Tests/AspireOracleManagedDataAccessExtensionsTests.cs

+
+public class AspireOracleManagedDataAccessExtensionsTests
+{
+    private const string ConnectionString = "user id=system;password=123;data source=localhost:1522/freepdb1";


Is this a well known connection string?

eerhardt · 2023-11-27T23:56:11Z

src/Components/Aspire.OracleManagedDataAccess/Aspire.OracleManagedDataAccess.csproj

@@ -0,0 +1,22 @@
+<Project Sdk="Microsoft.NET.Sdk">


The project/folder/package name should be Aspire.Oracle.ManagedDataAccess.Core.

Done. I ended up closing the PR unintentionally during this process, sorry about that

eerhardt · 2023-11-27T23:57:28Z

src/Components/Aspire.OracleManagedDataAccess/OracleHealthCheck.cs

+using Oracle.ManagedDataAccess.Client;
+
+namespace Aspire.OracleManagedDataAccess;
+internal sealed class OracleHealthCheck : IHealthCheck


Can we use https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/blob/master/src/HealthChecks.Oracle instead?

eerhardt · 2023-11-27T23:59:04Z

src/Components/Aspire.OracleManagedDataAccess/Aspire.OracleManagedDataAccess.csproj

@@ -0,0 +1,22 @@
+<Project Sdk="Microsoft.NET.Sdk">


Can you also update:

https://github.com/dotnet/aspire/blob/main/src/Components/Telemetry.md

https://github.com/dotnet/aspire/blob/main/src/Components/Aspire_Components_Progress.md

Add a README.

See other PRs for example:

Add Aspire.MySqlConnector #825

Azure cosmosdb support in aspire #359

Directory.Packages.props

eerhardt · 2023-11-28T00:08:06Z

src/Components/Aspire.OracleManagedDataAccess/AspireOracleManagedDataAccessExtensions.cs

+/// </summary>
+public static class AspireOracleManagedDataAccessExtensions
+{
+    private const string DefaultConfigSectionName = "Aspire:OracleManagedDataAccess";


Suggested change

private const string DefaultConfigSectionName = "Aspire:OracleManagedDataAccess";

private const string DefaultConfigSectionName = "Aspire:Oracle:ManagedDataAccess:Core";

This should follow the name of the component.

eerhardt · 2023-11-28T00:09:01Z

src/Components/Aspire.OracleManagedDataAccess/AspireOracleManagedDataAccessExtensions.cs

+        if (serviceKey is null)
+        {
+            // delay validating the ConnectionString until the DataSource is requested. This ensures an exception doesn't happen until a Logger is established.
+            builder.Services.AddTransient((serviceProvider) =>


The connection should be transient? I would have guessed scoped.

I agree, in this case it really is better for the connection to be scoped

eerhardt · 2023-11-28T00:10:03Z

src/Components/Aspire.OracleManagedDataAccess/OracleManagedDataAccessSettings.cs

+    public bool Tracing { get; set; } = true;
+
+    /// <summary>
+    /// <para>Gets or sets a boolean value that indicates whether the Open Telemetry metrics are enabled or not.</para>
+    /// <para>Enabled by default.</para>
+    /// </summary>
+    public bool Metrics { get; set; } = true;


Are these used? If not, we shouldn't have properties for them.

Removed for now, I looked in the ODP.NET documentation and there are still no metrics that we can use here.

eerhardt · 2023-11-28T00:10:54Z

src/Components/Aspire.OracleManagedDataAccess/ConfigurationSchema.json

@@ -0,0 +1,34 @@
+{
+  "properties": {


Is there integration with Logging? If so, can we add the log categories to the ConfigurationSchema.json?

See #430 for example.

Not for the time being. I believe that if they implement DbDataSource they will provide this feature.

FYI: oracle/dotnet-db-samples#329

mitchdenny · 2023-11-29T01:37:39Z

@andrevlins did you mean to close this?

smitpatel · 2023-11-29T01:38:55Z

Force push erased all the commits which were here so github closed the PR. 😟

andrevlins · 2023-11-29T01:57:30Z

@mitchdenny I'll reopen with the new folder structure

eerhardt · 2023-12-14T18:00:22Z

@andrevlins - this PR is marked as a Draft. Is it ready for review?

andrevlins · 2023-12-15T18:45:18Z

@andrevlins - this PR is marked as a Draft. Is it ready for review?

This PR depends on a version of the Oracle.ManagedDataAccess nuget package that is still in preview.
The main reason is that it is the version that supports Open Telemetry.

#1004 (comment)

eerhardt · 2024-01-18T15:31:18Z

@andrevlins - do you have a timeline for when you think this PR will be ready for review?

andrevlins · 2024-01-20T17:55:52Z

do you have a timeline for when you think this PR will be ready for review?

I am waiting for the stable version of the library Oracle.ManagedDataAccess.Core and Oracle.ManagedDataAccess.OpenTelemetry version 23.x.x due to OpenTelemetry support.

Some paths we can follow:

Continue with the PR in draft until the release of version 23.x.x.
Use the ```dev```` version and post the PR for review (I don't think working with unstable versions is viable in this project).
Downgrade the version to 3.21.xxx and not support OpenTelemetry (leaving this for another PR once the version is released).
Close the PR so as not to disturb your management and open it again when the version is released.

What do you think?

eerhardt · 2024-01-22T15:32:34Z

I'm inclined to say that if no progress is being made on this PR, let's close it for now. We can always re-open it when progress is ready to be made.

andrevlins · 2024-01-22T23:15:22Z

I'm inclined to say that if no progress is being made on this PR, let's close it for now. We can always re-open it when progress is ready to be made.

I'll do it this way then

ghost added the area-integrations Issues pertaining to Aspire Integrations packages label Nov 23, 2023

davidfowl reviewed Nov 24, 2023

View reviewed changes

mitchdenny reviewed Nov 27, 2023

View reviewed changes

eerhardt self-assigned this Nov 27, 2023

eerhardt reviewed Nov 28, 2023

View reviewed changes

andrevlins closed this Nov 28, 2023

andrevlins force-pushed the aspire-oracle branch from f9cd28f to 69f4227 Compare November 28, 2023 23:50

andrevlins and others added 2 commits November 28, 2023 23:02

apply reviews comments

3356076

Merge branch 'dotnet:main' into aspire-oracle

922345a

andrevlins reopened this Nov 29, 2023

readme.md and Aspire_Components_Progress.md

63637b4

joperezr added the community-contribution Indicates that the PR has been added by a community member label Dec 11, 2023

andrevlins mentioned this pull request Dec 12, 2023

Adds Aspire Oracle EntityFrameworkCore Database component. #1295

Merged

mitchdenny added this to the future (would take a change but work not scheduled) milestone Jan 22, 2024

andrevlins closed this Jan 22, 2024

andrevlins mentioned this pull request Mar 15, 2024

Remove dependencies on OpenTelemetry.Instrumentation.EntityFrameworkCore #2814

Closed

github-actions bot locked and limited conversation to collaborators Apr 28, 2024

	private const string DefaultConfigSectionName = "Aspire:OracleManagedDataAccess";
	private const string DefaultConfigSectionName = "Aspire:Oracle:ManagedDataAccess:Core";

Conversation

andrevlins commented Nov 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrevlins commented Nov 23, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mitchdenny commented Nov 29, 2023

Uh oh!

smitpatel commented Nov 29, 2023

Uh oh!

andrevlins commented Nov 29, 2023

Uh oh!

eerhardt commented Dec 14, 2023

Uh oh!

andrevlins commented Dec 15, 2023

Uh oh!

eerhardt commented Jan 18, 2024

Uh oh!

andrevlins commented Jan 20, 2024

Uh oh!

eerhardt commented Jan 22, 2024

Uh oh!

andrevlins commented Jan 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

andrevlins commented Nov 23, 2023 •

edited

Loading