Add WithRedisInsight #5227

Alirexaa · 2024-08-08T08:50:04Z

Description

In this PR, I Added Redis Insight, a Developer GUI for Redis, by Redis.

by using WithRedisInsight the RedisInsight instance starts and imports the Redis connection to this instance via HTTP call.

only some unit tests need to include this PR.

Fixes #5165

Checklist

Is this feature complete?
- Yes. Ready to ship.
- No. Follow-up changes expected.
Are you including unit tests for the changes and scenario tests if relevant?
- Yes
- No
Did you add public API?
- Yes
  - If yes, did you have an API Review for it?
    - Yes
    - No
  - Did you add <remarks /> and <code /> elements on your triple slash comments?
    - Yes
    - No
- No
Does the change make any security assumptions or guarantees?
- Yes
  - If yes, have you done a threat model and had a security review?
    - Yes
    - No
- No
Does the change require an update in our Aspire docs?
- Yes
  - Link to aspire-docs issue: Mention WithRedisInsight in Redis docs docs-aspire#1474
- No

Microsoft Reviewers: Open in CodeFlow

Alirexaa · 2024-08-08T12:50:33Z

To pass all tests we need to import docker.io/redis/redisinsight:2.54 to netaspireci.azurecr.io

Alirexaa · 2024-08-09T18:09:37Z

@eerhardt could you please import docker.io/redis/redisinsight:2.54 to make ci green?

src/Aspire.Hosting.Redis/RedisInsightConfigWriterHook.cs

eerhardt · 2024-08-15T19:51:52Z

src/Aspire.Hosting.Redis/RedisInsightConfigWriterHook.cs

+
+            await pipeline.ExecuteAsync(async (ctx) =>
+            {
+                var response = await client.PostAsync(apiUrl, content, ctx)


Is this the only way we can configure Redis Insight? I'm a bit concerned about sending HTTP requests from the AppHost. Can we instead write a file to a bind mount, like other places?

AFIK there is no way to configure Redis Insight from file. There is some open issue about this redis/RedisInsight#2785.
I could'nt find any better way.

tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs

radical · 2024-08-15T20:42:28Z

tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs

+                var testConnectionResponse = await client.GetAsync(getDatabasesApiUrl, cts.Token)
+                    .ConfigureAwait(false);
+                response.EnsureSuccessStatusCode();


If this fails then it would be useful to throw an exception with the url that failed.

tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs

radical · 2024-08-15T20:49:03Z

Should this be added to the TestShop playground app too like RedisCommander?

aspire/playground/TestShop/TestShop.AppHost/Program.cs

Lines 12 to 15 in 6f0e998

    
           basketCache.WithRedisCommander(c => 
        
                                { 
        
                                    c.WithHostPort(33801); 
        
                                });

Alirexaa · 2024-08-16T09:00:13Z

@eerhardt could you please import docker.io/redis/redisinsight:2.54 to make ci green?

@eerhardt

eerhardt · 2024-08-16T16:04:56Z

@eerhardt could you please import docker.io/redis/redisinsight:2.54 to make ci green?

I have pushed the image to the netaspireci.azurecr.io registry, but I don't believe the test will pass because we don't have Docker Desktop installed on the CI machines.

eerhardt · 2024-08-16T16:08:52Z

src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs

+                }
+                var connectionFilePath = Path.Combine(connectionFileDirectoryPath, "RedisInsight_connections.json");
+
+                using (var stream = new FileStream(connectionFilePath, FileMode.OpenOrCreate))


Do we need to write this out to a file? Can't we just keep it in memory and then send it via HTTP?

I did that first and it did not work but I'm not sure that I did that right

Agree with @eerhardt. Not sure why this needs to be a file. You just need a MemoryStream.

Alirexaa · 2024-08-16T16:38:38Z

@eerhardt could you please import docker.io/redis/redisinsight:2.54 to make ci green?

I have pushed the image to the netaspireci.azurecr.io registry, but I don't believe the test will pass because we don't have Docker Desktop installed on the CI machines.

What can I do for that?

radical · 2024-08-16T18:24:16Z

/azp run

azure-pipelines · 2024-08-16T18:24:28Z

Azure Pipelines successfully started running 1 pipeline(s).

Alirexaa · 2024-09-14T06:10:26Z

@Alirexaa are you going to get back to this one?

Yes. I will work on this.

src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs

Co-authored-by: David Fowler <[email protected]>

tests/Aspire.Hosting.Redis.Tests/AddRedisTests.cs

Co-authored-by: David Fowler <[email protected]>

src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs

davidfowl · 2024-09-15T05:22:38Z

tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs

            MaxRetryAttempts = 10,
        }).Build();

        await pipeline.ExecuteAsync(async (ct) =>


You should be able to remove this now right?

I think so.

For removing this i think we should use waitfor for Redis Insight

@davidfowl Redis Insight doesn't log anything useful for using WaitForText so I just used ResourceNotificationService to ensure the resource is running.

tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs

Co-authored-by: David Fowler <[email protected]>

davidfowl · 2024-09-15T16:40:04Z

I cloned and played with this a bit:

These logs show up when you use redis insight since you're using the HttpClient factory.

info: System.Net.Http.HttpClient.Default.LogicalHandler[100]
      Start processing HTTP request POST http://localhost:33802/api/databases/import
info: System.Net.Http.HttpClient.Default.ClientHandler[100]
      Sending HTTP request POST http://localhost:33802/api/databases/import

Another thing that's kind of annoying is accepting the eula on every launch:

Alirexaa · 2024-09-15T17:42:21Z

I cloned and played with this a bit:

These logs show up when you use redis insight since you're using the HttpClient factory.
info: System.Net.Http.HttpClient.Default.LogicalHandler[100]
      Start processing HTTP request POST http://localhost:33802/api/databases/import
info: System.Net.Http.HttpClient.Default.ClientHandler[100]
      Sending HTTP request POST http://localhost:33802/api/databases/import

Do you want us to use WaitForText for these logs?

Another thing that's kind of annoying is accepting the eula on every launch:

There is another API to accept the eula:
redis/RedisInsight#3267 (comment)

curl -XPATCH 'http://localhost:5540/api/settings' \
  -H 'Content-Type: application/json' \
  --data-raw '{"agreements":{"eula":true,"analytics":true,"notifications":true,"encryption":false}}

I will use this API to accept the eula.

src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs

davidfowl · 2024-09-17T18:17:50Z

src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs

+            using (var stream = new MemoryStream())
+            {
+                using var writer = new Utf8JsonWriter(stream);
+
+                writer.WriteStartObject();
+
+                writer.WritePropertyName("agreements");
+                writer.WriteStartObject();
+                writer.WriteBoolean("eula", true);
+                writer.WriteBoolean("analytics", false);
+                writer.WriteBoolean("notifications", false);
+                writer.WriteBoolean("encryption", false);
+                writer.WriteEndObject();
+
+                writer.WriteEndObject();
+
+                await writer.FlushAsync(ct).ConfigureAwait(false);
+                stream.Seek(0, SeekOrigin.Begin);
+                string json = Encoding.UTF8.GetString(stream.ToArray());
+                var content = new StringContent(json, Encoding.UTF8, "application/json");


If you want to write a follow up @Alirexaa, this can be simplified using JsonContent and JsonObject.

Add WithRedisInsight

a8266cb

ghost added the area-integrations Issues pertaining to Aspire Integrations packages label Aug 8, 2024

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Aug 8, 2024

Alirexaa mentioned this pull request Aug 8, 2024

Mention WithRedisInsight in Redis docs dotnet/docs-aspire#1474

Closed

Alirexaa added 3 commits August 8, 2024 15:26

Add unit tests

0d0f65c

Add functionl test

df3d287

revert test container registery

c2c5564

Merge branch 'dotnet:main' into WithRedisInsight

7b4c2f2

Merge branch 'dotnet:main' into WithRedisInsight

4c6fb2a

build-analysis bot mentioned this pull request Aug 10, 2024

Some tests timing out on helix #5247

Closed

eerhardt reviewed Aug 15, 2024

View reviewed changes

radical reviewed Aug 15, 2024

View reviewed changes

tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs Outdated Show resolved Hide resolved

radical reviewed Aug 15, 2024

View reviewed changes

tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs Outdated Show resolved Hide resolved

radical reviewed Aug 15, 2024

View reviewed changes

tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs Show resolved Hide resolved

Merge branch 'dotnet:main' into WithRedisInsight

6c7ecbb

Alirexaa added 3 commits August 16, 2024 12:35

Use Eventing API

ac2350a

Adress PR feedback

80a2e38

fix build

67bb195

build-analysis bot mentioned this pull request Aug 16, 2024

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Closed

3 tasks

eerhardt reviewed Aug 16, 2024

View reviewed changes

radical mentioned this pull request Aug 16, 2024

Tracking issue for testing tasks blocked due to no docker desktop available on CI #5325

Open

7 tasks

Alirexaa added 2 commits September 14, 2024 19:00

Merge branch 'main' into WithRedisInsight

7f8f7fd

Address PR feedback

a70276a

davidfowl reviewed Sep 14, 2024

View reviewed changes

src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs Outdated Show resolved Hide resolved

davidfowl reviewed Sep 14, 2024

View reviewed changes

src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs Outdated Show resolved Hide resolved

Apply suggestions from code review

5cf971b

Co-authored-by: David Fowler <[email protected]>

davidfowl reviewed Sep 14, 2024

View reviewed changes

tests/Aspire.Hosting.Redis.Tests/AddRedisTests.cs Outdated Show resolved Hide resolved

Update tests/Aspire.Hosting.Redis.Tests/AddRedisTests.cs

ef6e8dd

Co-authored-by: David Fowler <[email protected]>

davidfowl reviewed Sep 14, 2024

View reviewed changes

src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs Outdated Show resolved Hide resolved

davidfowl and others added 2 commits September 14, 2024 14:53

Update src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs

0a0fb8b

Fix build and test

8549e2c

davidfowl reviewed Sep 15, 2024

View reviewed changes

tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs Outdated Show resolved Hide resolved

Alirexaa and others added 3 commits September 15, 2024 09:04

Update tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs

2ae5cd3

Co-authored-by: David Fowler <[email protected]>

Address PR feedback

f3fa22e

nit

3bfdac9

Add accept eula

5ac764f

mitchdenny mentioned this pull request Sep 16, 2024

Add an "AcceptEula" API to containers that use environment variables to signal EULA acceptance. #5727

Open

Alirexaa added 3 commits September 16, 2024 22:27

Add manual eula acceptance

90b21b9

Merge branch 'dotnet:main' into WithRedisInsight

eac485b

Update Redis playground app

ccbbdbb

davidfowl reviewed Sep 17, 2024

View reviewed changes

src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs Outdated Show resolved Hide resolved

Adress PR feedback

3d506fc

davidfowl approved these changes Sep 17, 2024

View reviewed changes

davidfowl merged commit e8b6e5f into dotnet:main Sep 17, 2024

davidfowl reviewed Sep 17, 2024

View reviewed changes

Alirexaa deleted the WithRedisInsight branch September 17, 2024 18:19

github-actions bot locked and limited conversation to collaborators Oct 18, 2024

Add WithRedisInsight #5227

Add WithRedisInsight #5227

Uh oh!

Conversation

Alirexaa commented Aug 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Microsoft Reviewers: Open in CodeFlow

Uh oh!

Alirexaa commented Aug 8, 2024

Uh oh!

Alirexaa commented Aug 9, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

radical commented Aug 15, 2024

Uh oh!

Alirexaa commented Aug 16, 2024

Uh oh!

eerhardt commented Aug 16, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidfowl Sep 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Alirexaa commented Aug 16, 2024

Uh oh!

radical commented Aug 16, 2024

Uh oh!

azure-pipelines bot commented Aug 16, 2024

Uh oh!

Alirexaa commented Sep 14, 2024

Uh oh!

Uh oh!

Uh oh!

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!

Alirexaa Sep 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

davidfowl commented Sep 15, 2024

Uh oh!

Alirexaa commented Sep 15, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Alirexaa commented Aug 8, 2024 •

edited

Loading

davidfowl Sep 9, 2024 •

edited

Loading

Alirexaa Sep 15, 2024 •

edited

Loading