Skip to content
This repository was archived by the owner on Sep 27, 2023. It is now read-only.

Add caching client v3#43

Merged
shoe116 merged 2 commits intodevelopmentfrom
add-caching-client-v3
Nov 5, 2021
Merged

Add caching client v3#43
shoe116 merged 2 commits intodevelopmentfrom
add-caching-client-v3

Conversation

@akiomik
Copy link
Copy Markdown
Contributor

@akiomik akiomik commented Nov 5, 2021

No description provided.

@akiomik akiomik added the enhancement New feature or request label Nov 5, 2021
@akiomik akiomik requested a review from shoe116 November 5, 2021 09:23
@akiomik akiomik self-assigned this Nov 5, 2021
import { default as quarter } from '~/__mocks__/fixtures/v3/quarter.js'

export class BuffettCodeApiClientV2 {
export class BuffettCodeApiClientV3 {
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.

ここはtypoです…

Copy link
Copy Markdown
Contributor

@shoe116 shoe116 left a comment

Choose a reason for hiding this comment

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

@akiomik 実装としてはLGTM だけど、 cache class の切り方は相談したい

import { default as quarter } from '~/__mocks__/fixtures/v3/quarter.js'

export class BuffettCodeApiClientV2 {
export class BuffettCodeApiClientV3 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

これは今までの class 名がよろしくないのね

@@ -0,0 +1,23 @@
import { DateParam } from '~/fiscal-periods/date-param'

export class DailyCache {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ここ、resource 毎にcache class 分ける今みたいな実装と、 end_point + params で cache の key を作って cache としては1つにするか 2つやり方がありそう。

excel の方は後者の作戦だね、リソース管理がめんどくさいから。

Copy link
Copy Markdown
Contributor

@shoe116 shoe116 left a comment

Choose a reason for hiding this comment

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

大丈夫そう、 LY, LQ を渡すときの cache がちょっとどうするか考えどこだけど、まぁこれでいいか。
cacheを使うっていうのそういうことだな、「最新かどうかは保証しないけど、早くレスポンスを返す」

@shoe116 shoe116 merged commit 9793d66 into development Nov 5, 2021
@akiomik akiomik mentioned this pull request Nov 6, 2021
@akiomik akiomik deleted the add-caching-client-v3 branch November 10, 2021 06:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants