Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Inject sdk cache configuration to agents #856

Conversation

dkeysil
Copy link
Contributor

@dkeysil dkeysil commented Mar 12, 2024

No description provided.

// These values will be injected into the agent container to configure SDK
// SDKRequestTimeout timeout until the SDK must fallback to the RPC Node
// Value in seconds and can be a float.
SDKRequestTimeout = "5"
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rename this to SDKRequestTimeoutSeconds and make it 20 if the chain data bucket interval is 10s. Also I think I would exclude the word SDK from any naming - the bot could be based on anything. Maybe BotCacheRequestTimeout kind of naming could be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Fixed. Thanks

SDKRequestTimeout = "5"
// SDKRequestInterval interval between SDK requests
// Value in seconds and can be a float.
SDKRequestInterval = "1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's append Seconds to the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Fixed. Thanks

config/env.go Outdated

EnvJsonRpcHost = "JSON_RPC_HOST"
EnvJsonRpcPort = "JSON_RPC_PORT"
EnvCacheJsonRpcCachePort = "CACHE_JSON_RPC_PORT"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we prefix all of these new cache env vars like JSON_RPC_CACHE_*? E.g. JSON_RPC_CACHE_PORT, JSON_RPC_CACHE_REQUEST_TIMEOUT etc. Prefixing with CACHE could be confusing if we introduce another cache for the bots to use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Fixed. Thanks

@dkeysil dkeysil force-pushed the kisel/forta-1608-inject-caching-configuration-to-bots branch from e3386d5 to 62efc2b Compare March 12, 2024 17:09
@dkeysil dkeysil requested a review from canercidam March 12, 2024 17:10
@dkeysil dkeysil assigned dkeysil and unassigned aomerk Mar 12, 2024
@dkeysil dkeysil requested a review from aomerk March 12, 2024 17:11
@dkeysil dkeysil merged commit ce1e967 into kisel/forta-1609-accept-json-rpc-cache-requests-from-bots Mar 14, 2024
4 checks passed
@dkeysil dkeysil deleted the kisel/forta-1608-inject-caching-configuration-to-bots branch March 14, 2024 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants