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

Dynamically load strategies #167

Merged
merged 18 commits into from
Dec 6, 2023
Merged

Dynamically load strategies #167

merged 18 commits into from
Dec 6, 2023

Conversation

Adamantios
Copy link
Collaborator

No description provided.

Multiple strategies can be included in a single file, therefore, one hash may be mapped to more than one names.
@Adamantios Adamantios added the enhancement New feature or request label Dec 1, 2023
@Adamantios Adamantios requested a review from 0xArdi December 1, 2023 17:29
@Adamantios Adamantios force-pushed the feat/dynamic-strategies branch from 145ea9d to e372fd7 Compare December 4, 2023 18:35
@Adamantios Adamantios changed the title [WIP]: Dynamically load strategies Dynamically load strategies Dec 5, 2023
@Adamantios Adamantios marked this pull request as ready for review December 5, 2023 13:46
Comment on lines +101 to +110
if STRATEGY_RUN_METHOD in globals():
del globals()[STRATEGY_RUN_METHOD]
exec(self.strategy_exec, globals()) # pylint: disable=W0122 # nosec
method = globals().get(STRATEGY_RUN_METHOD, None)
if method is None:
self.context.logger.error(
f"No {STRATEGY_RUN_METHOD!r} method was found in {self.params.trading_strategy} strategy's executable."
)
return {BET_AMOUNT_FIELD: 0}
return method(*args, **kwargs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

So basically the assumption here is that the strategy doesnt block. I guess its fine for now, although maybe it makes sense to run this on a separate thread/process to not block the agent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Opened #169.

return
for strategy, file_hash in self.shared_state.strategy_to_filehash.items():
self.context.logger.info(f"Fetching {strategy} strategy...")
ipfs_msg, message = self._build_ipfs_get_file_req(file_hash)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do this? this class has access to the IPFS helper methods, you can use generators for it and avoid having callbacks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah this is an overkill, not sure why I chose this path.
I opened #170 as this works for now.

IpfsHandler = BaseIpfsHandler


class IpfsHandler(AbstractResponseHandler):
Copy link
Collaborator

Choose a reason for hiding this comment

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

you could just use the default IPFS handler, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct. Will be addressed in #170.

@@ -1,25 +0,0 @@
# -*- coding: utf-8 -*-
# ------------------------------------------------------------------------------
#
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to keep that and add at least one test so we can later build on it. Rather than deleting it now and adding it back in later.

Copy link
Collaborator Author

@Adamantios Adamantios Dec 6, 2023

Choose a reason for hiding this comment

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

Done in 7225bc3.

@Adamantios Adamantios force-pushed the feat/dynamic-strategies branch from 73567ce to 3f07634 Compare December 6, 2023 10:44
@Adamantios Adamantios merged commit 10d14ac into refactor Dec 6, 2023
6 checks passed
@Adamantios Adamantios deleted the feat/dynamic-strategies branch December 6, 2023 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants