#692 Simplify downstream service config #1353

cezarypiatek · 2020-10-13T18:33:49Z

Closes #692

This allows to simplify downstream configuration by defining a given downstream service once in the global configuration and reuse it later by referencing with GlobalHostKey.

{
  "Routes": [{
      "DownstreamPathTemplate": "/api/products/{productId}",
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "GlobalHostKey": "JsonPlaceholderService"
        }
      ],
      "UpstreamPathTemplate": "/products/{productId}",
      "UpstreamHttpMethod": [ "Put" ],
      "QoSOptions": {
        "ExceptionsAllowedBeforeBreaking": 3,
        "DurationOfBreak": 10,
        "TimeoutValue": 5000
      },
      "FileCacheOptions": { "TtlSeconds": 15 }
    },
    {
      "DownstreamPathTemplate": "/posts",
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "GlobalHostKey": "JsonPlaceholderService"
        }
      ],
      "UpstreamPathTemplate": "/posts/",
      "UpstreamHttpMethod": [ "Get" ],
      "QoSOptions": {
        "ExceptionsAllowedBeforeBreaking": 3,
        "DurationOfBreak": 10,
        "TimeoutValue": 5000
      },
      "FileCacheOptions": { "TtlSeconds": 15 }
    },
    {
      "DownstreamPathTemplate": "/",
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "Host": "www.bbc.co.uk",
          "Port": 80
        }
      ],
      "UpstreamPathTemplate": "/bbc/",
      "UpstreamHttpMethod": [ "Get" ]
    }
  ],

  "GlobalConfiguration": {
    "RequestIdKey": "ot-traceid",
    "DownstreamHosts": {
      "JsonPlaceholderService": {
        "Host": "jsonplaceholder.typicode.com",
        "Port": 80
      }
    } 
  }
}

This also allows to very easily override downstream address per environment.

Important: I've changed a lot of files because I need to rename FileHostAndPort → FileDownstreamHostConfig.
All things related to rename are in a separated commit.

Bonsai12345 · 2020-10-21T09:31:36Z

I can't believe this is not possible already. The docu says "The Global configuration is a bit hacky and allows overrides of Route specific settings." That sounds like it's exactly for this purpose. Is there a workaround for this prior to this change?

cezarypiatek · 2020-10-21T11:02:04Z

I didn't find anything that could allow for that. The Global Settings seems to be quite limited

Bonsai12345 · 2020-10-21T16:03:29Z

I didn't find anything that could allow for that. The Global Settings seems to be quite limited

Me neither sadly, this would be very useful.

gao-artur · 2020-11-20T07:02:16Z

#692 (comment)

raman-m · 2023-07-15T22:31:39Z

@cezarypiatek Hi Cezary!
Thanks for your interest in Ocelot!
Your PR is just great!

Unfortunately a lot of thing have changed during 3 years, and your feature branch contains merge conflicts which are impossible to resolve.

Could you Sync fork please? So, new develop branch will occur with all top commits!
After that, create new feature branch from develop one, and cherry pick required commits from current branch.

Syncing fork will ask you to resolve merge conflicts of this branch.
Give a try!

…reference it in route config

This reverts commit 6ac363c.

raman-m · 2023-08-10T17:54:47Z

@cezarypiatek Hi Cezary!
I thought you might need help resolving conflicts.
The feature branch has been rebased onto ThreeMammals:develop successfully!
The build is green.
So, we can start the code review...

I don't see develop branch in your fork! Your fork is too old.

Could you Sync fork please? So, new develop branch will occur with all top commits!

Could you add me as collaborator to your forked repo please? I will create develop branch and make it default.

raman-m · 2023-08-10T18:01:35Z

@mariotacke Hi Mario!
As an author of the #692 issue,
Could you review the current solution please?

@NicoJuicy @philproctor @gao-artur @golavr
After your discussions of the issue #692 , could you join to code review sessions too?

NicoJuicy

First PR review on Ocelot, but PR looks good.

Just an addition, the current assumption is that the key is defined within the same GlobalConfiguration file.

I think functionality-wise, it would simplify a lot if you could reference a setting in app.settings. Instead of referencing a named host in the same ocelot config.

Eg. Current: JsonPlaceholderService
Alternative: '/ProxySettings/FeatureFlagManagerEndpoint'

This setting would be looked up in:
appsettings.json
{
"ProxySettings":{ "FeatureFlagManagerEndpoint":"https://myfeatureflagmanager.domain.com"}
}

This however would expect a URL, instead of the combined host + port that seems to be standard within Ocelot.

raman-m · 2023-08-18T11:48:20Z

@NicoJuicy reviewed on Aug 11:

I think functionality-wise, it would simplify a lot if you could reference a setting in app.settings. Instead of referencing a named host in the same ocelot config.

Nico,
Thanks for your review!
Your idea is good! We have some problems with configuration JSON.
There are a lot of feature requests which would like to change Configuration feature: 👇

So, your idea to use appsettings.json for custom properties will allow to make the pipeline extendable and more stable.
Current ocelot.json design is a source of problems. This Configuration feature was designed by Tom. 😄
I believe migration to appsettings.json will solve a long list of issues, but such kind of refactoring requires a large efforts.

NicoJuicy · 2023-08-18T12:37:37Z

@raman-m I actually like the configuration how it is.
I have separate files per feature ( products, clients,
.. ) the only thing that is annoying is that I have to repeat my endpoints ( I use it as a gateway for a pwa on top of a modular monolith)

The ocelot implementation in the same app avoids cors issues.

Here, backendgateway is a setting in app.settings

I'm using an extension for that

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;

namespace Shop.Backend.Ocelot
{
    using global::Ocelot.Configuration.File;
    using Microsoft.Extensions.Configuration;
    using Microsoft.Extensions.DependencyInjection;
    public class GlobalHosts : Dictionary<string, Uri> { }
    public static class FileConfigurationExtensions
    {
        public static IServiceCollection ConfigureDownstreamHostAndPortsPlaceholders(
            this IServiceCollection services,
            IConfiguration configuration)
        {
            services.PostConfigure<FileConfiguration>(fileConfiguration =>
            {
                var globalHosts = configuration
                    .GetSection($"Ocelot:{nameof(FileConfiguration.GlobalConfiguration)}:Hosts")
                    .Get<GlobalHosts>();

                foreach (var route in fileConfiguration.Routes)
                {
                    ConfigureRoute(route, globalHosts);
                }
            });

            return services;
        }

        private static void ConfigureRoute(FileRoute route, GlobalHosts globalHosts)
        {
            foreach (var hostAndPort in route.DownstreamHostAndPorts)
            {
                var host = hostAndPort.Host;
                if (host.StartsWith("{") && host.EndsWith("}"))
                {
                    var placeHolder = host.TrimStart('{').TrimEnd('}');

                    if (globalHosts.TryGetValue(placeHolder, out var uri))
                    {
                        route.DownstreamScheme = uri.Scheme;
                        hostAndPort.Host = uri.Host;
                        hostAndPort.Port = uri.Port;
                    }
                }
            }
        }
    }
}

raman-m

@cezarypiatek
Hi Cezary!
We should keep in mind that class renaming is quite aggressive refactoring, and we must avoid such refactoring in favor of using composition and inheritance.

raman-m · 2023-08-18T11:55:38Z

src/Ocelot/Configuration/File/FileHostAndPort.cs

Renaming from FileHostAndPort to FileGlobalDownstreamHostConfig is just crazy idea!

raman-m · 2023-08-18T12:00:57Z

src/Ocelot/Configuration/File/FileRoute.cs

@@ -48,7 +48,7 @@ public FileRoute()
        public FileRateLimitRule RateLimitOptions { get; set; }
        public FileAuthenticationOptions AuthenticationOptions { get; set; }
        public FileHttpHandlerOptions HttpHandlerOptions { get; set; }
-        public List<FileHostAndPort> DownstreamHostAndPorts { get; set; }
+        public List<FileDownstreamHostConfig> DownstreamHostAndPorts { get; set; }


There is no need to rename the property to indicate that it contains configuration for downstream service route. It is totally clear thing!

raman-m · 2023-08-18T12:07:08Z

src/Ocelot/Configuration/File/FileDownstreamHostConfig.cs

+    public string Host { get; set; }
+    public int Port { get; set; }


These properties can be inherited from old existing FileHostAndPort class.

raman-m · 2023-08-18T12:09:26Z

src/Ocelot/Configuration/File/FileDownstreamHostConfig.cs

+public class FileDownstreamHostConfig
+{
+    /// <summary>
+    /// Key to reference downstream host config from global configuration.
+    /// </summary>
+    /// <value>
+    /// Key reference from <see cref="FileGlobalConfiguration.DownstreamHosts"/>.
+    /// </value>
+    public string GlobalHostKey { get; set; }


Suggested change

public class FileDownstreamHostConfig

{

/// <summary>

/// Key to reference downstream host config from global configuration.

/// </summary>

/// <value>

/// Key reference from <see cref="FileGlobalConfiguration.DownstreamHosts"/>.

/// </value>

public string GlobalHostKey { get; set; }

public class FileDownstreamHostConfig : FileHostAndPort

{

public string GlobalHostKey { get; set; }

}

raman-m · 2023-08-18T12:13:13Z

src/Ocelot/Configuration/File/FileGlobalDownstreamHostConfig.cs

Just zero benefits in defining of this new model!
You can reuse the old FileHostAndPort model for global config logic.

raman-m · 2023-08-18T12:33:29Z

test/Ocelot.ManualTest/ocelot.json

I guess it is better to make new copied ocelot2.json from this file and write new test(s) which will test exactly this new feature.

raman-m · 2023-08-18T12:38:51Z

test/Ocelot.UnitTests/Configuration/DownstreamAddressesCreatorTests.cs

+
+            var globalConfig = new FileGlobalConfiguration
+            {
+                DownstreamHosts = new Dictionary<string, FileGlobalDownstreamHostConfig>


Values should have the FileHostAndPort type actually!

Suggested change

DownstreamHosts = new Dictionary<string, FileGlobalDownstreamHostConfig>

DownstreamHosts = new Dictionary<string, FileHostAndPort>

raman-m · 2023-08-18T12:53:09Z

src/Ocelot/Configuration/Creator/DownstreamAddressesCreator.cs

+            _logger = loggerFactory.CreateLogger<DownstreamAddressesCreator>();
+        }
+
+        public List<DownstreamHostAndPort> Create(FileRoute route, FileGlobalConfiguration globalConfiguration)


We have to keep the version with one argument, and we have to add new overloaded version to the IDownstreamAddressesCreator interface!

raman-m · 2023-08-18T12:56:13Z

src/Ocelot/Configuration/Creator/DownstreamAddressesCreator.cs

-            return route.DownstreamHostAndPorts.Select(hostAndPort => new DownstreamHostAndPort(hostAndPort.Host, hostAndPort.Port)).ToList();
+            foreach (var downstreamHost in route.DownstreamHostAndPorts)
+            {
+                if (string.IsNullOrWhiteSpace(downstreamHost.GlobalHostKey) == false)


You don't read nullable type, it is safe to remove this equality operator.

raman-m · 2023-08-18T13:00:55Z

src/Ocelot/Configuration/Creator/IDownstreamAddressesCreator.cs

@@ -6,6 +6,6 @@ namespace Ocelot.Configuration.Creator
 {
    public interface IDownstreamAddressesCreator
    {
-        List<DownstreamHostAndPort> Create(FileRoute route);
+        List<DownstreamHostAndPort> Create(FileRoute route, FileGlobalConfiguration globalConfiguration);


We have to keep the version with one argument, and we have to add new overloaded version with 2 arguments!

raman-m · 2023-08-18T13:34:12Z

@NicoJuicy commented on Aug 18

Bad formatting of the description! It is hard to read!
I guess we must discuss the PR but not your custom solutions. You could open any issues in backlog to other custom features.

raman-m · 2023-08-18T15:05:32Z

@cezarypiatek commented on Oct 13, 2020:

Important: I've changed a lot of files because I need to rename FileHostAndPort → FileDownstreamHostConfig.
All things related to rename are in a separated commit.

Yeah, it is bad idea! See code review plz!
We should not rename classes, and should not provide any breaking changes at all!

This also allows to very easily override downstream address per environment.

What Ocelot feature do we need to use to achieve this? 😉

This allows to simplify downstream configuration by defining a given downstream service once in the global configuration and reuse it later by referencing with GlobalHostKey.

{
  "Routes": [{
      "DownstreamPathTemplate": "/api/products/{productId}",
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "GlobalHostKey": "JsonPlaceholderService"
        }
      ],
      "UpstreamPathTemplate": "/products/{productId}",
      "UpstreamHttpMethod": [ "Put" ],
    },
    {
      "DownstreamPathTemplate": "/posts",
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "GlobalHostKey": "JsonPlaceholderService"
        }
      ],
      "UpstreamPathTemplate": "/posts/",
      "UpstreamHttpMethod": [ "Get" ],
    },
    {
      "DownstreamPathTemplate": "/",
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "Host": "www.bbc.co.uk",
          "Port": 80
        }
      ],
      "UpstreamPathTemplate": "/bbc/",
      "UpstreamHttpMethod": [ "Get" ]
    }
  ],

  "GlobalConfiguration": {
    "RequestIdKey": "ot-traceid",
    "DownstreamHosts": {
      "JsonPlaceholderService": {
        "Host": "jsonplaceholder.typicode.com",
        "Port": 80
      }
    } 
  }
}

More short version:

{
  "Routes": [{
      "DownstreamHostAndPorts": [
        {
          "GlobalHostKey": "JsonPlaceholderService"
        }
      ],
    },
  ],

  "GlobalConfiguration": {
   "DownstreamHosts": {
      "JsonPlaceholderService": {
        "Host": "jsonplaceholder.typicode.com",
        "Port": 80
      }
    } 
  }
}

What is simplified here? You just removed one line from the "DownstreamHostAndPorts" property...
I understand simplification like that:

{
  "Routes": [{
      "DownstreamHostAndPorts": ["JsonPlaceholderService"],
    },
  ],

  "GlobalConfiguration": {
   "DownstreamHosts": {
      "JsonPlaceholderService": {
        "Host": "jsonplaceholder.typicode.com",
        "Port": 80
      }
    } 
  }
}

...which looks more simplified.

But, because "DownstreamHostAndPorts" property has described in a lot of docs, it is totally crazy idea to refactor this property by introducing breaking changes and confusion in docs!
It seems we have to design a new property for route to achieve real simplification.

{
  "Routes": [{
      "DownstreamHosts": ["JsonPlaceholderService"],
    },
  ],
  "GlobalConfiguration": {
   "DownstreamHosts": {
      "JsonPlaceholderService": {
        "Host": "jsonplaceholder.typicode.com",
        "Port": 80
      }}}
}

Moreover, I suggest you to introduce Scheme property in alias-object:

{
  "Routes": [
    // old definition
    {
      "DownstreamPathTemplate": "/posts",
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "Host": "jsonplaceholder.typicode.com",
          "Port": 80
        }
      ],
    },

    // new definition 1
    {
      "DownstreamPathTemplate": "/posts",
      "DownstreamScheme": "http",
      "DownstreamHosts": ["JsonPlaceholderService"],
    },

    // new definition 2
    {
      "DownstreamPathTemplate": "/posts",
      "DownstreamAlias": "JsonPlaceholderService", // not array!!!
    },
  ],

  "GlobalConfiguration": {
   "DownstreamHosts": {
      "JsonPlaceholderService": {
        "Host": "jsonplaceholder.typicode.com",
        "Port": 80
      }},
   "DownstreamAliases": {
      "JsonPlaceholderService": {
        "Scheme": "http",
        "Host": "jsonplaceholder.typicode.com",
        "Port": 80
      }}
  }
}

Pay attention "DownstreamAlias" of a route can be just single string, not an array object.
@cezarypiatek Looks simplistic?
@mariotacke Looks good?

Finally, because "DownstreamAliases" contains all required props of an URL, we can define alias like that:

{
  "Routes": [
    // old definition
    {
      "DownstreamPathTemplate": "/posts",
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "Host": "jsonplaceholder.typicode.com",
          "Port": 80
        }
      ],
    },

    // new definition
    {
      "DownstreamPathTemplate": "/posts",
      "DownstreamUrl": "JsonPlaceholderService", // not array!!!
    },
  ],

  "GlobalConfiguration": {
   "DownstreamUrls": {
      "JsonPlaceholderService": "http://jsonplaceholder.typicode.com",
    }
  }
}

But it seems such simplification by aliases is overhead design, because route can have simplest URL definition:

{
  "Routes": [
    // old definition
    {
      "DownstreamPathTemplate": "/posts",
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "Host": "jsonplaceholder.typicode.com",
          "Port": 80
        }
      ],
    },

    // new definition
    {
      "DownstreamPathTemplate": "/posts",
      "DownstreamUrl": "http://jsonplaceholder.typicode.com",
      // and/or
      "Downstream": "http://jsonplaceholder.typicode.com",
    },
  ],

  "GlobalConfiguration": {
    // empty
  }
}

No global aliases at all. And global config is empty. And we use default port 80 of the scheme!

ggnaegi · 2023-10-04T07:39:09Z

@raman-m

{
      "DownstreamPathTemplate": "/posts",
      "DownstreamScheme": "http",
      "DownstreamHosts": ["JsonPlaceholderService"],
},

This is an interesting PR, separating Routes and Hosts is a wise choice... We could imagine splitting the config file and with the help of an IHostedService retrieve the clusters dynamically. Then we would achieve a real polling 😄

raman-m · 2023-10-06T15:20:17Z

@ggnaegi commented on Oct 4

Gui, I don't know... This PR is about placeholders in global section, and this PR is not about dynamic routing, or service discovery...
To be fair, it will be hard to work on this PR...
It seems, as an author, @cezarypiatek doesn't care about his PR anymore. 😢 I've asked him a lot of questions, but he doesn't care... I cannot develop ideas of other developers when they don't respond, don't write code...

cezarypiatek · 2023-10-07T09:54:23Z

@ggnaegi commented on Oct 4

Gui, I don't know... This PR is about placeholders in global section, and this PR is not about dynamic routing, or service discovery... To be fair, it will be hard to work on this PR... It seems, as an author, @cezarypiatek doesn't care about his PR anymore. 😢 I've asked him a lot of questions, but he doesn't care... I cannot develop ideas of other developers when they don't respond, don't write code...

Correct, I don't care. I needed that 3 years ago.

ggnaegi · 2023-10-08T08:23:00Z

@raman-m It's still a very interesting PR though. A "neat" but breaking solution would be to use the ServiceName we would use for consul or eureka and then allow the host definition in GlobalConfiguration.

raman-m · 2023-10-30T09:55:23Z

@cezarypiatek commented on Oct 7

Strange answer... Good luck!

FYI, I have been leading this project since May 2023, started 6 months ago...

raman-m · 2023-10-30T09:57:04Z

@ggnaegi commented on Oct 7

Let's decide what to do with this PR after October's release...

raman-m assigned cezarypiatek Jul 15, 2023

raman-m changed the base branch from master to develop July 15, 2023 22:12

raman-m added the conflicts Feature branch has merge conflicts label Jul 15, 2023

raman-m added invalid Not actually an issue needs feedback Issue is waiting on feedback before acceptance waiting Waiting for answer to question or feedback from issue raiser and removed invalid Not actually an issue labels Jul 15, 2023

cezarypiatek and others added 3 commits August 10, 2023 19:45

ThreeMammals#692: Rename class keeping downstream service config

ed2560b

ThreeMammals#692: Add option to define downstream hosts globally and …

0b1dc91

…reference it in route config

SA1629 Documentation text should end with a period

f4d9633

raman-m force-pushed the feature/692_simplify_downstream_service_config branch from 7d40231 to f4d9633 Compare August 10, 2023 17:07

raman-m added 3 commits August 10, 2023 20:24

Convert to file-scoped namespace

6ac363c

Revert "Convert to file-scoped namespace"

a39193d

This reverts commit 6ac363c.

File-scoped namespace

211eb08

raman-m added feature A new feature and removed waiting Waiting for answer to question or feedback from issue raiser conflicts Feature branch has merge conflicts labels Aug 10, 2023

raman-m mentioned this pull request Aug 10, 2023

DownstreamHostAndPorts alias #692

Open

raman-m requested review from binarymash, jps, geffzhang, RaynaldM and philproctor August 10, 2023 18:12

raman-m requested review from anktsrkr, sharpn, abooraja, wast, raman-m, yangzhongke and thiagoloureiro August 10, 2023 18:12

RaynaldM approved these changes Aug 11, 2023

View reviewed changes

NicoJuicy reviewed Aug 11, 2023

View reviewed changes

raman-m requested changes Aug 18, 2023

View reviewed changes

raman-m added the MVP Minimum Viable Product label Aug 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#692 Simplify downstream service config #1353

#692 Simplify downstream service config #1353

cezarypiatek commented Oct 13, 2020 •

edited by raman-m

Bonsai12345 commented Oct 21, 2020

cezarypiatek commented Oct 21, 2020

Bonsai12345 commented Oct 21, 2020

gao-artur commented Nov 20, 2020

raman-m commented Jul 15, 2023

raman-m commented Aug 10, 2023

raman-m commented Aug 10, 2023

NicoJuicy left a comment

raman-m commented Aug 18, 2023

NicoJuicy commented Aug 18, 2023 •

edited by raman-m

raman-m left a comment

raman-m Aug 18, 2023

raman-m Aug 18, 2023

raman-m Aug 18, 2023

raman-m Aug 18, 2023

raman-m Aug 18, 2023

raman-m Aug 18, 2023

raman-m Aug 18, 2023

raman-m Aug 18, 2023

raman-m Aug 18, 2023

raman-m Aug 18, 2023

raman-m commented Aug 18, 2023

raman-m commented Aug 18, 2023 •

edited

ggnaegi commented Oct 4, 2023 •

edited by raman-m

raman-m commented Oct 6, 2023 •

edited

cezarypiatek commented Oct 7, 2023

ggnaegi commented Oct 8, 2023

raman-m commented Oct 30, 2023 •

edited

raman-m commented Oct 30, 2023

		public string Host { get; set; }
		public int Port { get; set; }

	DownstreamHosts = new Dictionary<string, FileGlobalDownstreamHostConfig>
	DownstreamHosts = new Dictionary<string, FileHostAndPort>

#692 Simplify downstream service config #1353

Are you sure you want to change the base?

#692 Simplify downstream service config #1353

Conversation

cezarypiatek commented Oct 13, 2020 • edited by raman-m

Closes #692

Bonsai12345 commented Oct 21, 2020

cezarypiatek commented Oct 21, 2020

Bonsai12345 commented Oct 21, 2020

gao-artur commented Nov 20, 2020

raman-m commented Jul 15, 2023

raman-m commented Aug 10, 2023

raman-m commented Aug 10, 2023

NicoJuicy left a comment

Choose a reason for hiding this comment

raman-m commented Aug 18, 2023

NicoJuicy commented Aug 18, 2023 • edited by raman-m

raman-m left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raman-m commented Aug 18, 2023

raman-m commented Aug 18, 2023 • edited

ggnaegi commented Oct 4, 2023 • edited by raman-m

raman-m commented Oct 6, 2023 • edited

cezarypiatek commented Oct 7, 2023

ggnaegi commented Oct 8, 2023

raman-m commented Oct 30, 2023 • edited

raman-m commented Oct 30, 2023

cezarypiatek commented Oct 13, 2020 •

edited by raman-m

NicoJuicy commented Aug 18, 2023 •

edited by raman-m

raman-m commented Aug 18, 2023 •

edited

ggnaegi commented Oct 4, 2023 •

edited by raman-m

raman-m commented Oct 6, 2023 •

edited

raman-m commented Oct 30, 2023 •

edited