Require explicit user-consent to enable HadoopFileIO #1532

snazy · 2025-05-06T12:14:32Z

Using HadoopFileIO in Polaris can enable "hidden features" that users are likely not aware of. This change requires users to manually update the configuration to be able to use HadoopFileIO in way that highlights the consequences of enabling it.

This PR updates Polaris in multiple ways:

The default of SUPPORTED_CATALOG_STORAGE_TYPES is changed to not include the FILE storage type.
Respect the ALLOW_SPECIFYING_FILE_IO_IMPL configuration on namespaces, tables and views to prevent setting an io-impl value for anything but one of the configured, supported storage-types.
Unify validation code in a new class IcebergPropertiesValidation.
Using FILE or HadoopFileIO now also requires the explicit configuration ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS=true.
Added production readiness checks that trigger when ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS is true or SUPPORTED_CATALOG_STORAGE_TYPES contains FILE (defaults and per-realm).
The two new readiness checks are considered severe. Severe readiness-errors prevent the server from starting up - unless the user explicitly configured polaris.readiness.ignore-security-issues=true.

Log messages and configuration options explicitly use "clear" phrases highlighting the consequences.

With these changes it is intentionally extremely difficult to start Polaris with HadoopFileIO. People who work around all these safety nets must have realized that what they are doing.

A lot of the test code relies on FILE/HadoopFileIO, those tests got all the configurations to let those tests continue to work as they are, bypassing the added security safeguards.

dimas-b · 2025-05-06T12:59:28Z

...kus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java

          "polaris.features.defaults.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"",
          "true",
          "polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"",
-          "[\"FILE\"]");
+          "[\"FILE\",\"S3\"]",
+          "polaris.readiness.ignore-security-issues",


WDYT about putting this into a global quarkus test config profile?

Yea - but I guess all Quarkus profiles should either be special per-test or unified/composable or moved to a central place.

composition works by default... one can have application.properties and profile classes at the same time and their properties are merged, right?

Yea - but let's defer to a separate PR to clean all those up.

eric-maynard · 2025-05-06T17:13:46Z

This is not a bug.

eric-maynard

There's a lot going on with this PR and I think it might be good to separate out some of these changes. For example, just removing FILE as a default supported storage type would be valuable.

polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java

eric-maynard · 2025-05-06T17:26:53Z

polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java

+          .defaultValue(false)
+          .buildFeatureConfiguration();
+
+  public static final FeatureConfiguration<Boolean>


I don't see how this isn't completely redundant with SUPPORTED_CATALOG_STORAGE_TYPES

It's there to make the fact mentioned in the description explicit.

If you put FILE in SUPPORTED_CATALOG_STORAGE_TYPES that seems quite explicit no?

@eric-maynard You are correct in that this flag is redundant. However, I believe the intent here is to make the consequences 100% explicit.

I can see how some newcomers may overlook the SUPPORTED_CATALOG_STORAGE_TYPES=[..., "FILE"] during a configuration review and not realize the consequences it could have. Whereas an environment variable that contains "insecure" and "security risks" will definitely raise eyebrows.

Think about deleting a Github repository where you have to go to the Danger Zone, click "Delete, confirm, then confirm a second time, then type the repository name and confirm a third time". Here, given the consequences, I think it is worth having a double-confirmation of the FILE storage type.

I have a hard time believing we can't incrementalize this change the evaluate the necessity of the individual components of it. We're going from this being in the default config for multiple releases to requiring multiple flags to enable it.

AFAICT, because HadoopFileIO is considered insecure, we want to make it hard for users to do the wrong thing. Hence the addition of multiple flags. With the current way, it is trivial to accidentally overlook the activation of HadoopFileIO.

There could be other proposals for this implementation. But then they will be evaluated against this one goal: make it hard for users to do the wrong thing.

I think you are (and maybe this PR is) conflating two things -- FILE storage type and HadoopFileIO. They are not always the same.

SUPPORTED_CATALOG_STORAGE_TYPES is probably sufficient to disallow the FILE storage type. FILE shouldn't be in the default, but I think it's too great a leap to go from having it in the default to requiring multiple flags to enable it.

For disallowing HadoopFileIO, we need a different approach than what's done in this PR.

Concrete suggestions for improvements are always welcome.

However, this PR addresses some issue that should really be mitigated quickly.

eric-maynard · 2025-05-06T17:28:36Z

...rvice/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java

@@ -55,26 +62,53 @@ public class ProductionReadinessChecks {
   */
  private static final String WARNING_SIGN_UTF_8 = "\u0000\u26A0\uFE0F";

+  private static final String SEVERE_SIGN_UTF_8 = "\u0000\uD83D\uDED1";


These changes seem pretty unnecessary

I do think that there are nuances in what's a warning and what's severe.

That may be so, but I don't think this change is necessary to Require explict user-consent to enable HadoopFileIO

@eric-maynard I am not sure what the ask is here. Would you rather have the UTF-8 icon and/or the log message implemented in a separate PR? Or do you mean that the PR title should be a bit more general?

Sure, I think a separate PR for these changes could be a good idea. If this PR really is a bugfix, let's not couple unnecessary changes with it.

Let's be mindful on the overhead that too fine-grained PR would cause though...

The changes in this PR are cohesive to one another. And it is quite small. So it makes sense to submit the changes in a single PR. Splitting a PR into many tiny PRs would prevent reviewers from understanding the big picture and would not have any functional merit.

As I think the changes are "related", I think it's fair to do the changes in one PR (even if it's just "display/rendering" messages).

It's definitely fair, it's only a question of necessity. If there really is an urgent 1.0-blocking bugfix to be made, must we include these changes here? Are these logging changes really 1.0-blocking?

eric-maynard · 2025-05-06T17:31:11Z

...src/main/java/org/apache/polaris/service/catalog/validation/IcebergPropertiesValidation.java

+          && !configStore
+              .getConfiguration(ctx, SUPPORTED_CATALOG_STORAGE_TYPES)
+              .contains(storageType.name())) {


Storage types and File IO implementations are not necessarily the same thing; you could have any number of FileIO implementations that can talk to S3

Yes? That's what fromFileIoImplementation() figures out, no?

For the hard-coded FileIO implementations (assuming the implementation you have in mind is actually the one on the classpath), yes. For other implementations, no.

@eric-maynard could you provide an example of what you mean?

There's one in the repo here. It uses Azure, but it's not represented in StorageTypeFileIO. How is that meant to behave?

eric-maynard · 2025-05-06T17:31:29Z

...src/main/java/org/apache/polaris/service/catalog/validation/IcebergPropertiesValidation.java

+          && !configStore.getConfiguration(
+              ctx, ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS)) {
+        throw new ValidationException(
+            "File IO implementation '%s' (storage type '%s') is considered insecure and must not be used",


What makes a custom File IO insecure?

What makes it secure?

Nothing. That's why there's nothing in our current documentation or code that promises that it is secure.

snazy · 2025-05-07T15:33:42Z

This is not a bug.

It is. And it is very severe.

eric-maynard · 2025-05-07T17:21:31Z

How exactly is this a bug?

flyrain · 2025-05-07T18:31:50Z

  S3("org.apache.iceberg.aws.s3.S3FileIO", true),
  GCS("org.apache.iceberg.gcp.gcs.GCSFileIO", true),
  AZURE("org.apache.iceberg.azure.adlsv2.ADLSFileIO", true),
  FILE("org.apache.iceberg.hadoop.HadoopFileIO", false),

Thanks for bring this up. It's a good point to clarify whether the HadoopFileIO is suitable for production. However, there is no guarantee any of above strings ensure a secure FileIO. They can be easily injected by any customized build or even completely different implementations with the same name. I believe it is ultimately Polaris admin's responsibility to make sure their Polaris deployment as secure as possible. We(Polaris developer/community) can provide suggestions to them for sure. With that, I'm +1 on well documented behavior on HadoopFileIO, but I don't think these checking and new configurations are necessary. SUPPORTED_CATALOG_STORAGE_TYPES should be good enough to let Polaris admin make the decision.

snazy · 2025-05-07T19:03:42Z

whether the HadoopFileIO is suitable for production

@flyrain you should really know why it's not

eric-maynard · 2025-05-09T17:49:55Z

...ce/common/src/main/java/org/apache/polaris/service/catalog/validation/StorageTypeFileIO.java

+  S3("org.apache.iceberg.aws.s3.S3FileIO", true),
+
+  GCS("org.apache.iceberg.gcp.gcs.GCSFileIO", true),
+
+  AZURE("org.apache.iceberg.azure.adlsv2.ADLSFileIO", true),
+
+  FILE("org.apache.iceberg.hadoop.HadoopFileIO", false),


We can't actually guarantee that the implementation found at this location by the classloader is the one you have in mind

That is true, but again, I'd suggest improving the "precision" of checks later, after merging this PR.

Feature configs are forever -- if we won't ever be able to make the check precise, we shouldn't ever add a config that implies a precise check. I don't see how the check can be made precise, so I am opposed to adding the config now.

Also, HadoopFileIO is used for more than just FILE -- it's the fallback defined here. In the past we've seen it used for AZURE paths.

eric-maynard · 2025-05-09T17:51:04Z

...ce/common/src/main/java/org/apache/polaris/service/catalog/validation/StorageTypeFileIO.java

+  static StorageTypeFileIO fromFileIoImplementation(String fileIoImplementation) {
+    var type = FILE_TO_TO_STORAGE_TYPE.get(fileIoImplementation);
+    if (type == null) {
+      throw new IllegalArgumentException("Unknown FileIO implementation: " + fileIoImplementation);


Is this expected to throw for a custom fileIO?

Good point. I guess this goes beyond the original intent behind this PR 🤔

@snazy : WDYT about deferring the enforcement of the compile-time list of FileIO implementations?

dimas-b · 2025-05-09T19:12:44Z

INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST

It was present before this PR, in IcebergCatalog

flyrain · 2025-05-10T01:34:07Z

INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST was already present, but I don’t think that justifies promoting it to a first-class config flag. In fact, we shouldn’t be exposing this at all, it’s a test-only utility and should be removed.
That said, I don’t think this PR needs to handle the removal. A more reasonable approach would be to leave it as-is for now and clean it up in a follow-up PR.
Let’s not legitimize it by formalizing it in the public config surface.

dimas-b · 2025-05-10T02:53:02Z

INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST was already present, but I don’t think that justifies promoting it to a first-class config flag. In fact, we shouldn’t be exposing this at all, it’s a test-only utility and should be removed.

Good points.

@dennishuo : is it removable now? IIRC it had some usecase before, but I'm not sure whether it is still needed 🤔

eric-maynard · 2025-05-10T08:05:43Z

INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST was already present, but I don’t think that justifies promoting it to a first-class config flag.

+1, it was left out of the original PolarisConfiguration change for a reason and this is also what I am saying here

As for removing it, I don't really have an opinion. It's certainly not a change that seems necessary to fix any kind of bug.

eric-maynard · 2025-05-10T08:14:53Z

...e/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusReadinessConfiguration.java

+   * risks.
+   */
+  @WithDefault("false")
+  boolean ignoreSevereIssues();


Besides what's already been pointed out about the validity of the FileIO check done here, this is not a good config name. What is a "severe issue"? Does it mean that we don't ignore mild issues? Are there super-severe issues?

@eric-maynard "Severe" is a standard term to denote the most important information. See java.util.logging.Level for an example of how it is used in the context of logs.

Having a log level of SEVERE and a config called "ignore severe issues" are completely different things. Is the implication that this config would disable failure in all cases where we use the SEVERE logging level?

Even if that is the case, my questions stand. Does this config not apply to less-severe issues? What guidelines will qualify an issue as severe (necessitating a SEVERE log)? Have we gone back and audited all "issues" to verify none should be considered SEVERE? What even is an "issue"?

@eric-maynard I don't know how to answer your questions. They seem related to a much bigger scope than the scope of this PR. Especially the audit part. We could extend the logic that in subsequent PRs, so that more cases are covered, if it is what you have in mind.

I guess my question to you is : how can we move this PR forward?

I think my comment lays out a path:

it might be good to separate out some of these changes

In the interest of moving things forward I linked some examples of what more incremental improvements might look like below

I am not sure it lays out a path, that's the thing. What value would be created out of splitting this PR into smaller PRs?

It could also possibly be a topic for a ML discussion if there are PR guidelines that are not explicitly written in CONTRIBUTING.md. Maybe I am missing something...?

We should aim for small independent code changes whenever possible. Just because two things work well together doesn't mean they should be in the same Pull request. As a generic guide, 1 "feature" per PR, small is better than bigger, independent changes should stay independent. IMHO, if the title can't cover the scope of the changes then the PR should be broken up. I think we have several independent things here as noted in the PR description. Ideally for me even refactors go in separate pull-requests. We've been a bit lax on this recently as we have adopted code from other projects but I don't think we should get in the habit.

The general benefits of this approach are for downstream integrators which can cherry-pick changes easier, and reviewers will have an easier time since net small changes are much easier to understand and usually easier to come to agreement on. (Bisecting also becomes much easier). If PR's can only be understood of a larger context, then grouping them in a project is usually the right thing to do or explaining the context in a design doc.

I think this pr itself does a great job of mapping out the independent changes in a bulleted list. IMHO, each of those points could probably approached independently since I think we already have consensus on at least some of the Items.

It's not about PR guidelines -- there are a bunch of issues with the code changes proposed in this PR that I've highlighted in various comments. Even the questions asked in this thread are not resolved. I still don't know what a "severe issue" is.

Breaking up the change could allow us to move forward with some of the less controversial aspects of the change, but if you're opposed to that I'm also happy to continue hashing out these issues here.

If I generally agree with the last comments, specifically in this PR, I consider the changes "related": several/rendering messages is related to clearly inform the users about allowing insecure file types.
So, it's fair to "group" related changes in one PR in this case.
Another option would have been to just focus on "warning messages" (not rendered) when allowing insecure file types. As I'm sure that 90% of the users won't use insecure file types (and use the default configuration), a warning (not seen 90% of the time) would be sufficient.

eric-maynard · 2025-05-12T09:09:55Z

See #1566 for a simple implementation removing FILE from the storage types supported by default.

See this branch for an example of a simple way to prohibit the default file IO factory from letting the service use a HadoopFileIO.

adutra · 2025-05-12T21:24:06Z

...rvice/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java

+                errors.add(
+                    Error.ofSevere(
+                        format(
+                            "The storage type '%s' is considered insecure and to expose the service to severe security ricks!",


Suggested change

"The storage type '%s' is considered insecure and to expose the service to severe security ricks!",

"The storage type '%s' is considered insecure and exposes the service to severe security risks!",

adutra · 2025-05-12T21:24:26Z

...rvice/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java

+                      errors.add(
+                          Error.ofSevere(
+                              format(
+                                  "The storage type '%s' is considered insecure and to expose the service to severe security ricks!",


Suggested change

"The storage type '%s' is considered insecure and to expose the service to severe security ricks!",

"The storage type '%s' is considered insecure and exposes the service to severe security risks!",

jbonofre · 2025-05-13T07:23:38Z

polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java

+          .defaultValue(false)
+          .buildFeatureConfiguration();
+
+  public static final FeatureConfiguration<Boolean> ALLOW_INSECURE_STORAGE_TYPES =


As this is a user facing property, we need to be consistent (hard to change later).
I think this property is good enough as it's generic to any storage type (not only FILE).

jbonofre · 2025-05-13T07:25:05Z

quarkus/defaults/src/main/resources/application.properties

@@ -109,7 +109,7 @@ polaris.realm-context.header-name=Polaris-Realm
 polaris.realm-context.require-header=false

 polaris.features.defaults."ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING"=false
-polaris.features.defaults."SUPPORTED_CATALOG_STORAGE_TYPES"=["S3","GCS","AZURE","FILE"]
+polaris.features.defaults."SUPPORTED_CATALOG_STORAGE_TYPES"=["S3","GCS","AZURE"]


Nit: For clarify, I wonder if we should not add polaris.features.defaults."ALLOW_INSECURE_STORAGE_TYPES"=false here.

jbonofre · 2025-05-13T07:26:27Z

...rvice/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java

@@ -55,26 +62,53 @@ public class ProductionReadinessChecks {
   */
  private static final String WARNING_SIGN_UTF_8 = "\u0000\u26A0\uFE0F";

+  private static final String SEVERE_SIGN_UTF_8 = "\u0000\uD83D\uDED1";


As I think the changes are "related", I think it's fair to do the changes in one PR (even if it's just "display/rendering" messages).

jbonofre · 2025-05-13T07:31:02Z

...e/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusReadinessConfiguration.java

+   * risks.
+   */
+  @WithDefault("false")
+  boolean ignoreSevereIssues();


If I generally agree with the last comments, specifically in this PR, I consider the changes "related": several/rendering messages is related to clearly inform the users about allowing insecure file types.
So, it's fair to "group" related changes in one PR in this case.
Another option would have been to just focus on "warning messages" (not rendered) when allowing insecure file types. As I'm sure that 90% of the users won't use insecure file types (and use the default configuration), a warning (not seen 90% of the time) would be sufficient.

flyrain · 2025-05-17T05:49:33Z

As I said in the email thread, I don't think ALLOW_INSECURE_STORAGE_TYPES is necessary, but I'm OK with it. With that, +0 on the PR.

Using `HadoopFileIO` in Polaris can enable "hidden features" that users are likely not aware of. This change requires users to manually update the configuration to be able to use `HadoopFileIO` in way that highlights the consequences of enabling it. This PR updates Polaris in multiple ways: * The default of `SUPPORTED_CATALOG_STORAGE_TYPES` is changed to not include the `FILE` storage type. * Respect the `ALLOW_SPECIFYING_FILE_IO_IMPL` configuration on namespaces, tables and views to prevent setting an `io-impl` value for anything but one of the configured, supported storage-types. * Unify validation code in a new class `IcebergPropertiesValidation`. * Using `FILE` or `HadoopFileIO` now _also_ requires the explicit configuration `ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS=true`. * Added production readiness checks that trigger when `ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS` is `true` or `SUPPORTED_CATALOG_STORAGE_TYPES` contains `FILE` (defaults and per-realm). * The two new readiness checks are considered _severe_. Severe readiness-errors prevent the server from starting up - unless the user explicitly configured `polaris.readiness.ignore-security-issues=true`. Log messages and configuration options explicitly use "clear" phrases highlighting the consequences. With these changes it is intentionally extremely difficult to start Polaris with HadoopFileIO. People who work around all these safety nets must have realized that what they are doing. A lot of the test code relies on `FILE`/`HadoopFileIO`, those tests got all the configurations to let those tests continue to work as they are, bypassing the added security safeguards.

@flyrain

Dismissing @flyrain's RFC due to his comment above and this mail.

snazy requested review from adutra, ashvina, dennishuo, dimas-b, eric-maynard, jackye1995, jbonofre, vvcephei and collado-mike as code owners May 6, 2025 12:14

github-project-automation bot added this to Basic Kanban Board May 6, 2025

snazy requested review from RussellSpitzer, takidau, MonkeyCanCode, flyrain, ebyhr and HonahX as code owners May 6, 2025 12:14

github-project-automation bot moved this to PRs In Progress in Basic Kanban Board May 6, 2025

dimas-b reviewed May 6, 2025

View reviewed changes

snazy force-pushed the explicit-hadoop-fio branch from 0a748ba to 3f65378 Compare May 6, 2025 13:42

snazy added bug Something isn't working 1.0-blocker labels May 6, 2025

eric-maynard removed the bug Something isn't working label May 6, 2025

eric-maynard requested changes May 6, 2025

View reviewed changes

snazy added the bug Something isn't working label May 7, 2025

snazy force-pushed the explicit-hadoop-fio branch from 3f65378 to c149a7f Compare May 8, 2025 14:11

eric-maynard reviewed May 9, 2025

View reviewed changes

snazy dismissed dimas-b’s stale review via a126bab May 9, 2025 18:04

snazy force-pushed the explicit-hadoop-fio branch from 0dc46a3 to a126bab Compare May 9, 2025 18:04

eric-maynard reviewed May 10, 2025

View reviewed changes

snazy mentioned this pull request May 12, 2025

Remove FILE from default SUPPORTED_CATALOG_STORAGE_TYPES #1566

Open

snazy force-pushed the explicit-hadoop-fio branch 2 times, most recently from e610053 to 23f49d2 Compare May 12, 2025 07:56

adutra reviewed May 12, 2025

View reviewed changes

jbonofre changed the title ~~Require explict user-consent to enable HadoopFileIO~~ Require explicit user-consent to enable HadoopFileIO May 13, 2025

jbonofre previously approved these changes May 13, 2025

View reviewed changes

dimas-b and others added 7 commits May 19, 2025 11:02

phrasing

79a5e40

rename one property

d7b3db7

rename one mth

1ede61b

property

26a73ff

move

77733a1

update test

6403276

snazy dismissed jbonofre’s stale review via 6403276 May 19, 2025 09:21

snazy force-pushed the explicit-hadoop-fio branch from 23f49d2 to 6403276 Compare May 19, 2025 09:21

snazy requested a review from singhpk234 as a code owner May 19, 2025 09:21

"defaults"

e951904

pingtimeout mentioned this pull request May 19, 2025

Site: Update production configuration page #1606

Merged

	"The storage type '%s' is considered insecure and to expose the service to severe security ricks!",
	"The storage type '%s' is considered insecure and exposes the service to severe security risks!",

Require explicit user-consent to enable HadoopFileIO #1532

Are you sure you want to change the base?

Require explicit user-consent to enable HadoopFileIO #1532

Conversation

snazy commented May 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimas-b May 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-maynard commented May 6, 2025

eric-maynard 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

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

eric-maynard May 13, 2025 • edited Loading

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

snazy commented May 7, 2025

eric-maynard commented May 7, 2025

flyrain commented May 7, 2025

snazy commented May 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-maynard May 10, 2025 • edited Loading

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

dimas-b commented May 9, 2025

flyrain commented May 10, 2025

dimas-b commented May 10, 2025

eric-maynard commented May 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-maynard May 12, 2025 • edited Loading

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

RussellSpitzer May 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-maynard commented May 12, 2025 • edited Loading

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

flyrain commented May 17, 2025

dimas-b May 6, 2025 •

edited

Loading

eric-maynard May 13, 2025 •

edited

Loading

eric-maynard May 10, 2025 •

edited

Loading

eric-maynard May 12, 2025 •

edited

Loading

RussellSpitzer May 12, 2025 •

edited

Loading

eric-maynard commented May 12, 2025 •

edited

Loading