#765 Add neverRepairPartitions option to skip MSCK REPAIR TABLE in Hive helpers.#766
Merged
Merged
Conversation
…Hive helpers.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughA new Hive configuration flag is added and threaded through metastore table-creation paths. Hive helper interfaces and both helper implementations now accept the flag and conditionally skip partition repair. Call sites and tests were updated accordingly. The SBT version was also updated. ChangesHive partition repair flag
Build tool update
Sequence Diagram(s)sequenceDiagram
participant appConfig
participant MetastoreImpl
participant HiveHelper
appConfig->>MetastoreImpl: read pramen.hive.never.repair.partitions
MetastoreImpl->>HiveHelper: createOrUpdateHiveTable(..., neverRepairPartitions)
alt partitionBy non-empty and neverRepairPartitions=false
HiveHelper->>HiveHelper: repairHiveTable()
else partitionBy empty or neverRepairPartitions=true
HiveHelper->>HiveHelper: log "Skipping repairing partition"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pramen/core/src/main/scala/za/co/absa/pramen/core/utils/hive/HiveHelper.scala (1)
31-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider a default value to avoid a source-breaking API change.
Adding
neverRepairPartitions: Booleanas a required parameter on these public abstract methods breaks every existing implementer and caller ofHiveHelper. Defaulting tofalsepreserves the prior behavior and backward compatibility.♻️ Proposed signature with default
def createHiveTable(path: String, format: HiveFormat, schema: StructType, partitionBy: Seq[String], databaseName: Option[String], tableName: String, - neverRepairPartitions: Boolean): Unit + neverRepairPartitions: Boolean = false): Unit def createOrUpdateHiveTable(path: String, format: HiveFormat, schema: StructType, partitionBy: Seq[String], databaseName: Option[String], tableName: String, - neverRepairPartitions: Boolean): Unit + neverRepairPartitions: Boolean = false): Unit🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pramen/core/src/main/scala/za/co/absa/pramen/core/utils/hive/HiveHelper.scala` around lines 31 - 40, The public HiveHelper API now adds a required neverRepairPartitions parameter on createOrUpdateHiveTable overloads, which breaks existing callers and implementers. Update the HiveHelper trait and any overriding implementations to give neverRepairPartitions a default of false so the prior behavior remains unchanged and the API stays backward compatible; use the createOrUpdateHiveTable symbols in HiveHelper.scala to locate the affected signatures.pramen/core/src/main/scala/za/co/absa/pramen/core/utils/hive/HiveHelperSparkCatalog.scala (1)
41-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated partition-repair gating block.
This exact
if (partitionBy.nonEmpty && !neverRepairPartitions) { ... } else { ... }block with the two skip-log messages is now repeated four times (here andcreateOrUpdateHiveTablein this file, plus both methods inHiveHelperSql). Extracting a small shared helper (e.g. in theHiveHelpercompanion takingfullTableName,partitionBy,neverRepairPartitions, and a repair thunk) would remove the duplication and keep the skip-logging consistent. Flagging once here; the same applies to the other three occurrences.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pramen/core/src/main/scala/za/co/absa/pramen/core/utils/hive/HiveHelperSparkCatalog.scala` around lines 41 - 48, The partition-repair gating logic in this block is duplicated in multiple places and should be centralized. Extract the repeated `if (partitionBy.nonEmpty && !neverRepairPartitions)` check and the two skip-log branches into a shared helper in the `HiveHelper` companion, and have `repairHiveTable` or a repair thunk passed in so `createOrUpdateHiveTable` and the `HiveHelperSql` methods can reuse it. Keep the helper responsible for the consistent skip logging using `fullTableName`, `partitionBy`, and `neverRepairPartitions`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@pramen/extras/src/main/scala/za/co/absa/pramen/extras/sink/EnceladusSink.scala`:
- Line 452: The `neverRepairPartitions` flag is hardcoded to false in
`EnceladusSink` and similarly in `StandardizationSink`, so these sinks ignore
the runtime `never.repair.partitions` setting. Update the
`createOrUpdateHiveTable` call sites to pass the configured value from the
sink’s runtime/config object instead of a literal, using the same wiring pattern
already exposed through `MetastoreImpl` and the persistence layer.
---
Nitpick comments:
In
`@pramen/core/src/main/scala/za/co/absa/pramen/core/utils/hive/HiveHelper.scala`:
- Around line 31-40: The public HiveHelper API now adds a required
neverRepairPartitions parameter on createOrUpdateHiveTable overloads, which
breaks existing callers and implementers. Update the HiveHelper trait and any
overriding implementations to give neverRepairPartitions a default of false so
the prior behavior remains unchanged and the API stays backward compatible; use
the createOrUpdateHiveTable symbols in HiveHelper.scala to locate the affected
signatures.
In
`@pramen/core/src/main/scala/za/co/absa/pramen/core/utils/hive/HiveHelperSparkCatalog.scala`:
- Around line 41-48: The partition-repair gating logic in this block is
duplicated in multiple places and should be centralized. Extract the repeated
`if (partitionBy.nonEmpty && !neverRepairPartitions)` check and the two skip-log
branches into a shared helper in the `HiveHelper` companion, and have
`repairHiveTable` or a repair thunk passed in so `createOrUpdateHiveTable` and
the `HiveHelperSql` methods can reuse it. Keep the helper responsible for the
consistent skip logging using `fullTableName`, `partitionBy`, and
`neverRepairPartitions`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ef99cd2-df99-462d-9bf9-ec364594ea88
📒 Files selected for processing (10)
pramen/core/src/main/resources/reference.confpramen/core/src/main/scala/za/co/absa/pramen/core/config/Keys.scalapramen/core/src/main/scala/za/co/absa/pramen/core/metastore/MetastoreImpl.scalapramen/core/src/main/scala/za/co/absa/pramen/core/utils/hive/HiveHelper.scalapramen/core/src/main/scala/za/co/absa/pramen/core/utils/hive/HiveHelperSparkCatalog.scalapramen/core/src/main/scala/za/co/absa/pramen/core/utils/hive/HiveHelperSql.scalapramen/core/src/test/scala/za/co/absa/pramen/core/tests/utils/hive/HiveHelperSparkCatalogSuite.scalapramen/core/src/test/scala/za/co/absa/pramen/core/tests/utils/hive/HiveHelperSqlSuite.scalapramen/extras/src/main/scala/za/co/absa/pramen/extras/sink/EnceladusSink.scalapramen/extras/src/main/scala/za/co/absa/pramen/extras/sink/StandardizationSink.scala
Unit Test Coverage
Files
|
…lChannelFactoryV2 and add default value to HiveHelper methods.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #765
Summary by CodeRabbit
New Features
pramen.hive.never.repair.partitionsto control whether partition repair is skipped during Hive table create/update (default:false).Bug Fixes
Tests
Chores