Skip to content

Commit fbee547

Browse files
authored
Add loggings and allow 0 as range start for s3 backfills (#438)
Revamp S3 range validation with detailed message and allow 0 as the range start value. For S3 Backfills - The range isn’t actually defaulting to 0 — what’s happening is that it’s cloning the values from the previous run. For example, [this run](https://backfila.sqprod.co/backfills/28700) has a range of 0 to null. So when cloned, the start value of 0 gets copied over. On the other hand, [this run](https://backfila.sqprod.co/backfills/28806) has both start and end as null, so those values are cloned instead. This means that if a previous run had a start value of 0, and the user selects RESTART, that 0 will be reused — which can will cause errors for S3 since valid range should not be there. To avoid this, We want to support a start value of 0 for S3 backfills. current logging user get - ![Screenshot 2025-04-17 at 2 21 46 PM](https://github.com/user-attachments/assets/00b768c2-bd6d-47dd-bed6-e98e193536ee)
1 parent 36bdcb6 commit fbee547

File tree

2 files changed

+5
-5
lines changed

2 files changed

+5
-5
lines changed

client-s3/src/main/kotlin/app/cash/backfila/client/s3/internal/S3DatasourceBackfillOperator.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@ class S3DatasourceBackfillOperator<R : Any, P : Any>(
2727
override fun prepareBackfill(request: PrepareBackfillRequest): PrepareBackfillResponse {
2828
val config = parametersOperator.constructBackfillConfig(request)
2929
backfill.validate(config)
30-
31-
require(request.range?.start == null && request.range?.end == null) {
30+
val isRangeValid = (request.range?.start?.utf8() == "0" || request.range?.start == null) && request.range?.end == null
31+
require(isRangeValid) {
3232
// We could think about supporting this later by making the range mean a byte seek into all S3 files.
3333
// This would mean we would need to support some kind of seek forward or seek back for record start.
3434
// That or perhaps we only support it for single file?
3535
// In any case, we are not implementing this now.
36-
"Range is currently unsupported for S3 Backfils"
36+
"Only full ranges are currently supported for S3 Backfills."
3737
}
3838

3939
val pathPrefix = backfill.getPrefix(config)

client-s3/src/test/kotlin/app/cash/backfila/client/s3/S3Utf8StringNewlineBackfillTest.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,12 @@ class S3Utf8StringNewlineBackfillTest {
112112
this.assertThatCode {
113113
backfila.createWetRun<BreakfastBackfill>(rangeStart = "a start")
114114
fail("invalid range must fail")
115-
}.hasMessageContaining("Range is currently unsupported")
115+
}.hasMessageContaining("Only full ranges are currently supported for S3 Backfills.")
116116

117117
this.assertThatCode {
118118
backfila.createWetRun<BreakfastBackfill>(rangeEnd = "a end")
119119
fail("invalid range must fail")
120-
}.hasMessageContaining("Range is currently unsupported")
120+
}.hasMessageContaining("Only full ranges are currently supported for S3 Backfills.")
121121

122122
this.assertThatCode {
123123
backfila.createWetRun<BrunchBackfill>()

0 commit comments

Comments
 (0)