More guardrails around ShouldDeleteFromName
#842
Conversation
lib/destination/ddl/expiry.go
Outdated
| nameParts := strings.Split(name, "_") | ||
| if len(nameParts) < 2 { | ||
| if len(nameParts) < 6 { |
There was a problem hiding this comment.
That's just how many are in a temp table
There was a problem hiding this comment.
I could put it back to 2 if it's confusing?
There was a problem hiding this comment.
Oh I see, because right after the table name there are 3 underscores in a row, so 2 of the 6 parts are empty. Maybe it would be more clear to just split the part that comes after the constants.ArtiePrefix and check for 2 parts there? Because otherwise if the table name itself has any underscores in it, we could get a false positive here.
There was a problem hiding this comment.
@danafallon Want to take another look?
lib/destination/ddl/expiry_test.go
Outdated
| tablesToNotDrop := []string{ | ||
| "foo", | ||
| "transactions", | ||
| fmt.Sprintf("expired_tbl__artie_suffix_%d", time.Now().Add(constants.TemporaryTableTTL).Unix()), |
There was a problem hiding this comment.
Call this future_tbl__... for clarity?
lib/destination/ddl/expiry.go
Outdated
| return false | ||
| } | ||
|
|
||
| unixString := nameParts[len(nameParts)-1] | ||
| unix, err := strconv.Atoi(unixString) | ||
| tsString := suffixParts[len(suffixParts)-1] |
There was a problem hiding this comment.
Could just hardcode this to [2] if we're enforcing that len(suffixParts) has to be exactly 3, but I think it's also fine as is.
Similar logic to
transfer/lib/destination/ddl/ddl.go
Line 22 in 060ed44