Skip to content

syncing fallback storage service redesign - #745

Merged
tsahee merged 11 commits into
masterfrom
syncing_fallback_storage
Jul 20, 2022
Merged

syncing fallback storage service redesign#745
tsahee merged 11 commits into
masterfrom
syncing_fallback_storage

Conversation

@tsahee

@tsahee tsahee commented Jul 1, 2022

Copy link
Copy Markdown
Contributor

No description provided.

@cla-bot cla-bot Bot added the cla-signed label Jul 1, 2022
@codecov

codecov Bot commented Jul 1, 2022

Copy link
Copy Markdown

Codecov Report

Merging #745 (d9ac160) into master (0b2d68e) will decrease coverage by 4.53%.
The diff coverage is 5.72%.

@@            Coverage Diff             @@
##           master     #745      +/-   ##
==========================================
- Coverage   52.88%   48.34%   -4.54%     
==========================================
  Files         222      210      -12     
  Lines       27139    23184    -3955     
  Branches      494      494              
==========================================
- Hits        14353    11209    -3144     
+ Misses      11242    10429     -813     
- Partials     1544     1546       +2     

@rachel-bousfield rachel-bousfield changed the title syncing fallback storage service redisign syncing fallback storage service redesign Jul 15, 2022
Tristan-Wilson
Tristan-Wilson previously approved these changes Jul 19, 2022

@Tristan-Wilson Tristan-Wilson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM minor comment about a comment only

Comment thread das/syncing_fallback_storage.go Outdated
return err
}
log.Info("BatchDelivered", "log", batchDeliveredLog, "event", deliveredEvent)
storeUntil := arbmath.SaturatingUAdd(deliveredEvent.TimeBounds.MaxTimestamp, uint64(s.config.RetentionPeriod.Seconds())) // TODO: support limited retention period

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the TODO comment here still correct? Isn't it limited if we're only adding the retention period?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

you're right :) will remove

@tsahee
tsahee merged commit 5e80745 into master Jul 20, 2022
@tsahee
tsahee deleted the syncing_fallback_storage branch July 20, 2022 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants