Fixing all lint errors by sofia-tekdatum · Pull Request #233 · protegrity/arrow

sofia-tekdatum · 2026-01-23T01:58:06Z

Fix all lint errors reported in github actions. See [this run] for the original errors. (https://github.com/protegrity/arrow/actions/runs/20860646944/job/59939104419).

Clean run (with a straggling R lint error which is not ours) is here.

github-actions · 2026-01-23T01:58:28Z

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

argmarco-tkd

Thank you for this! - this was massive (not a criticism!!). LGTM - it seemed mainly like a re-formatting PR, let me know if there is anything in particular that may need closer attention.

Now, future looking question: what could/should we have done before so that we didn't get to this point (where we had to put in massive effort to correct formatting)?

argmarco-tkd · 2026-01-25T03:41:19Z

-  catch (...) {
-    ARROW_LOG(ERROR) << "[DBPAExecutor] UNKNOWN EXCEPTION: " << operation_name << " failed with unknown exception";
-    throw; // Re-throw original exception
+  } catch (const std::exception& e) {


ew. but whatever it takes to make the linter happy :D

This one I do actually prefer :)

argmarco-tkd · 2026-01-25T03:45:45Z

-    {"port", "5432"},
-    {"database", "testdb"}
-  };
+      {"host", "localhost"}, {"port", "5432"}, {"database", "testdb"}};


sigh. not as legible as the original. oh, well.

argmarco-tkd · 2026-01-25T04:04:52Z

-  int64_t init_timeout_ms    = 10*1000; //10 seconds
-  int64_t encrypt_timeout_ms = 30*1000; //30 seconds
-  int64_t decrypt_timeout_ms = 30*1000; //30 seconds.
+  int64_t init_timeout_ms = 10 * 1000;     // 10 seconds


ugg, again :)

There were many, many uggs when I was doing this.

My internal linter likes this actually :)

sofia-tekdatum · 2026-01-25T15:43:48Z

Thank you for this! - this was massive (not a criticism!!). LGTM - it seemed mainly like a re-formatting PR, let me know if there is anything in particular that may need closer attention.

Now, future looking question: what could/should we have done before so that we didn't get to this point (where we had to put in massive effort to correct formatting)?

Thanks for the review! Going forward, every PR should make sure the linter action is clean before merging :)

avalerio-tkd

LGTM+

Thanks for all the changes @sofia-tekdatum ! Nothing major on the comments, except for one. If that's the only change, it's ok to leave as-is.

avalerio-tkd · 2026-01-25T19:48:47Z

+        {parquet::Type::DOUBLE, dbps::external::Type::DOUBLE},
+        {parquet::Type::BYTE_ARRAY, dbps::external::Type::BYTE_ARRAY},
+        {parquet::Type::FIXED_LEN_BYTE_ARRAY, dbps::external::Type::FIXED_LEN_BYTE_ARRAY},
+        {parquet::Type::UNDEFINED, dbps::external::Type::UNDEFINED}};


Probably a note for the future, but this one is likely to fail when we integrate the new dbps::external::Type without UNDEFINED.
Kind of a mental note, not related to this PR

@avalerio-tkd - I noticed this as well. We will deal with this once we bring the latest version of DBPA interface to Arrow. Not a biggie, and definitely not a blocker for this PR.

avalerio-tkd · 2026-01-25T19:49:56Z

+      parquet_to_external_type_map;
+  static const std::unordered_map<::arrow::Compression::type,
+                                  dbps::external::CompressionCodec::type>
+      arrow_to_external_compression_map;


ufff.. :( If this passes the linter, then fine.

avalerio-tkd · 2026-01-25T19:51:35Z

+   * @param encrypt_timeout Timeout for encrypt operations in milliseconds
+   *                        (default: 30000)
+   * @param decrypt_timeout Timeout for decrypt operations in milliseconds
+   * (default: 30000)


Nit: Align as the previous line for consistency. If this is the only change, then it's ok to leave as-is.

I am choosing to leave this as the linter is currently happy and I'll prioritize that.

avalerio-tkd · 2026-01-25T19:54:05Z

+            std::string app_context, std::string column_key_id, Type::type data_type,
+            std::optional<int> datatype_length, CompressionCodec::type compression_type,
+            std::optional<std::map<std::string, std::string>> column_encryption_metadata)
+      override {


My internal linter doesn't like this, but leave it as-is if this passes the Arrow linter. :(

avalerio-tkd · 2026-01-25T19:56:59Z

-  int64_t init_timeout_ms    = 10*1000; //10 seconds
-  int64_t encrypt_timeout_ms = 30*1000; //30 seconds
-  int64_t decrypt_timeout_ms = 30*1000; //30 seconds.
+  int64_t init_timeout_ms = 10 * 1000;     // 10 seconds


My internal linter likes this actually :)

avalerio-tkd · 2026-01-25T19:57:59Z

-                      std::move(meta_decryptor_factory),
-                      std::move(data_decryptor_factory)};
+    CryptoContext ctx{
+        col->has_dictionary_page(),        static_cast<int16_t>(row_group_ordinal_),


Fixing all lint errors

50e09bf

github-actions Bot added Component: C++ Component: Parquet Component: Python awaiting review labels Jan 23, 2026

sofia-tekdatum added 8 commits January 22, 2026 21:28

Fixing all lint errors

e338f70

Fixing all lint errors

e144bfc

Fix all lint errors

83dd902

Fix all lint errors

50e4060

Fixing linting errors

e0ad719

Fixing linting errors

eaf9c2d

Fixing linting errors

61f6d1b

Fixing linting errors

cd8e31a

sofia-tekdatum linked an issue Jan 23, 2026 that may be closed by this pull request

Git merging > Cleanup final failing Github actions #226

Closed

sofia-tekdatum requested review from argmarco-tkd and avalerio-tkd January 23, 2026 17:44

argmarco-tkd approved these changes Jan 25, 2026

View reviewed changes

github-actions Bot added awaiting committer review and removed awaiting review labels Jan 25, 2026

avalerio-tkd approved these changes Jan 25, 2026

View reviewed changes

sofia-tekdatum merged commit 953bcb7 into main Jan 26, 2026
45 of 115 checks passed

sofia-tekdatum deleted the fix_lint_errors branch January 26, 2026 16:32

Uh oh!

Conversation

sofia-tekdatum commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jan 23, 2026

Uh oh!

argmarco-tkd left a comment

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!

sofia-tekdatum commented Jan 25, 2026

Uh oh!

avalerio-tkd left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sofia-tekdatum commented Jan 23, 2026 •

edited

Loading