Skip to content

Conversation

@geruh
Copy link
Contributor

@geruh geruh commented Oct 24, 2025

Rationale for this change

Note: Opening this to verify auto-run CI behavior.

This also includes a small refactor to _convert_glue_to_iceberg. We were assigning parameters early and never using it. Since we only need specific values from Parameters, this change uses the walrus operator to read them inline and removes the unused variable.

Are these changes tested?

Yes

Are there any user-facing changes?

No

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the _convert_glue_to_iceberg method in the Glue catalog to use the walrus operator (:=) for inline assignment and validation. The changes eliminate an unused properties variable that was assigned but never referenced, and consolidate dictionary access with null checks using the walrus operator pattern.

Key Changes:

  • Removed unused properties variable assignment
  • Converted dictionary access patterns to use walrus operator with inline null checks
  • Changed direct key access (glue_table["Name"]) to safer .get() method with validation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks for the refactor

@kevinjqliu kevinjqliu merged commit a793fe5 into apache:main Oct 25, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants