Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve API friendliness? #11

Open
rtyley opened this issue Sep 15, 2023 · 0 comments
Open

Improve API friendliness? #11

rtyley opened this issue Sep 15, 2023 · 0 comments

Comments

@rtyley
Copy link
Member

rtyley commented Sep 15, 2023

This is how the ETagCache is declared in Frontend:

  private val pressedPageCache: ETagCache[ObjectId, PressedPage] = new ETagCache(
    S3ObjectFetching(S3Async, Bytes)
      .timing(
        successWith = FrontDownloadLatency.recordDuration,
        notModifiedWith = FrontNotModifiedDownloadLatency.recordDuration,
      )
      .thenParsing { bytes =>
        withMetrics(FrontDecodingLatency) {
          Using(new GZIPInputStream(bytes.asInputStream()))(Json.parse(_).as[PressedPage]).get
        }
      },
    AlwaysWaitForRefreshedValue,
    _.maximumSize(2000).expireAfterAccess(1.hour),
  )

Feedback from discussing this API with other devs (where it may have been a mistake- overwhelming? to discuss the innards of the library):

  • Mixing new and fluent builder interfaces was a little confusing. I think the preference was for new style construction everywhere, though I think that maybe be difficult for Loading, which simplifies its type signature to just 2 type parameters by not including the 3rd type parameter of 'response'
  • Dev wondered where is the 'response' type hiding? This diagram aided understanding:
    image

It might make sense to make AlwaysWaitForRefreshedValue the default, and thus remove one additional parameter.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant