Auto polygon has paid #476

grzesir · 2024-06-29T20:52:52Z

No description provided.

…ional messages

korbit-ai · 2024-06-29T20:52:56Z

My review is in progress 📖 - I will have feedback for you in a few minutes!

korbit-ai

I have reviewed your code and found 12 potential issues. To discuss my individual comments that I have added, tag me in replies using @korbit-ai.

Please react with a 👍 to my comments that you find helpful and a 👎 to those you find unhelpful - this will help me learn and improve as we collaborate.

korbit-ai · 2024-06-29T21:01:10Z

lumibot/example_strategies/options_hold_to_expiry.py

@@ -91,5 +91,4 @@ def on_trading_iteration(self):
            backtesting_end,
            benchmark_asset="SPY",
            polygon_api_key="YOUR_POLYGON_API_KEY_HERE",  # Add your polygon API key here


Security issue identified: The Polygon API key is hardcoded directly in the code as 'YOUR_POLYGON_API_KEY_HERE'. Hardcoding sensitive information like API keys is a security risk. If an attacker gains access to the codebase, they could extract the API key. To resolve this, remove the API key from the code and instead read it from an environment variable or a separate configuration file that is not committed to version control.

korbit-ai · 2024-06-29T21:01:11Z

lumibot/tools/polygon_helper.py

@@ -189,7 +196,7 @@ def get_price_data_from_polygon(

    return df_all

-def validate_cache(force_cache_update: bool, asset: Asset, cache_file: Path, api_key: str, paid: bool):
+def validate_cache(force_cache_update: bool, asset: Asset, cache_file: Path, api_key: str):


The 'validate_cache' function has been updated to remove the 'paid' parameter, but the function body still references a 'paid' variable. To avoid potential runtime errors, please update the function to work correctly without this parameter.

korbit-ai · 2024-06-29T21:01:12Z

lumibot/strategies/_strategy.py

@@ -1,6 +1,6 @@
 import datetime
 import logging
-import warnings
+from termcolor import colored
 from asyncio.log import logger


The import 'from asyncio.log import logger' appears to be unused and should be removed to clean up the code.

korbit-ai · 2024-06-29T21:01:13Z

lumibot/strategies/_strategy.py

@@ -773,7 +773,7 @@ def run_backtest(
        sell_trading_fees=[],
        api_key=None,
        polygon_api_key=None,
-        polygon_has_paid_subscription=False,
+        polygon_has_paid_subscription=None, # Depricated, this is now automatic. Remove in future versions.


There is a spelling mistake in the comment next to the polygon_has_paid_subscription parameter. It should be corrected from 'Depricated' to 'Deprecated' to maintain professionalism in the codebase.

korbit-ai · 2024-06-29T21:01:14Z

lumibot/tools/polygon_helper.py

+            colored_message = colored(
+                "Polygon rate limit reached. Sleeping for 1 minute before trying again. If you want to avoid this, consider a paid subscription with Polygon at https://polygon.io/?utm_source=affiliate&utm_campaign=lumi10 Please use the full link to give us credit for the sale, it helps support this project. You can use the coupon code 'LUMI10' for 10% off.",
+                "red",
+            )


The log message includes an affiliate link and a coupon code, which may not be suitable for logging purposes. Please remove these to keep the logs neutral and focused on operational information.

korbit-ai · 2024-06-29T21:01:16Z

lumibot/tools/polygon_helper.py

+                "Polygon rate limit reached. Sleeping for 1 minute before trying again. If you want to avoid this, consider a paid subscription with Polygon at https://polygon.io/?utm_source=affiliate&utm_campaign=lumi10 Please use the full link to give us credit for the sale, it helps support this project. You can use the coupon code 'LUMI10' for 10% off.",
+                "red",
+            )
+            logging.error(colored_message)


The current log message is unstructured. Please use structured logging with key-value pairs to improve log analysis capabilities.

korbit-ai · 2024-06-29T21:01:17Z

lumibot/strategies/_strategy.py

@@ -951,6 +948,17 @@ def run_backtest(
        if stats_file is None:
            stats_file = f"{logdir}/{basename}_stats.csv"

+        # Check if polygon_has_paid_subscription is set (it is deprecated and will be removed in the future)
+        if polygon_has_paid_subscription is not None:
+            colored_warning = colored("The parameter `polygon_has_paid_subscription` is deprecated and will be removed in the future. "


The use of termcolor for logging a deprecation warning within the run_backtest method may not be captured in certain test environments. Consider replacing it with standard logging practices to ensure that the deprecation warning can be captured and asserted in tests.

korbit-ai · 2024-06-29T21:01:18Z

lumibot/strategies/_strategy.py

@@ -1,6 +1,6 @@
 import datetime
 import logging
-import warnings
+from termcolor import colored


The use of 'termcolor' for coloring log messages is not recommended as it may cause compatibility issues across different environments. Consider using a logging configuration that supports colored output in a more controlled way, such as 'colorlog' or custom log formatters.

korbit-ai · 2024-06-29T21:01:19Z

lumibot/tools/polygon_helper.py



    def _get(self, *args, **kwargs):
        logging.info(
            f"\nSleeping {PolygonClient.WAIT_SECONDS} seconds while getting data from Polygon "
            "to avoid hitting the rate limit; "
-            "consider a paid Polygon subscription for faster results.\n"
+            "If you want to avoid this, consider a paid subscription with Polygon at https://polygon.io/?utm_source=affiliate&utm_campaign=lumi10 (please use the full link to give us credit for the sale, it helps support this project). You can use the coupon code 'LUMI10' for 10% off.\n"
        )
        time.sleep(PolygonClient.WAIT_SECONDS)


The hardcoded sleep calls to avoid hitting the Polygon API rate limit can hinder automated testing by making it slow. Consider refactoring to allow for dependency injection of the PolygonClient, enabling you to mock out the actual API calls during testing for faster execution.

korbit-ai · 2024-06-29T21:01:20Z

lumibot/tools/polygon_helper.py

-    force_cache_update = validate_cache(force_cache_update, asset, cache_file, api_key, has_paid_subscription)
-
+    force_cache_update = validate_cache(force_cache_update, asset, cache_file, api_key)
+    
    df_all = None


The variable name 'df_all' could be more descriptive to better indicate its purpose. Consider renaming it to 'combined_price_data' to clarify that it holds the combined data from cache and Polygon.

korbit-ai · 2024-06-29T21:01:21Z

lumibot/tools/polygon_helper.py

@@ -123,7 +120,7 @@ def get_price_data_from_polygon(
    # Check if we need to get more data
    missing_dates = get_missing_dates(df_all, asset, start, end)
    if not missing_dates:
-        # TODO: Do this upstream so we don't called repeatedly for known-to-be-missing bars.
+        # TODO: Do this upstream so we don't repeatedly call for known-to-be-missing bars.


The TODO comment in the get_price_data_from_polygon function is vague. Please clarify what action should be taken upstream and why it's necessary to avoid repeated calls for known-to-be-missing bars.

korbit-ai · 2024-06-29T21:01:22Z

setup.py

@@ -5,7 +5,7 @@

 setuptools.setup(
    name="lumibot",
-    version="3.5.8",
+    version="3.5.9",


I noticed that the version number of the package has been incremented in the setup.py file. However, there is no information in the pull request summary about what changes have been made that justify this version increment. Could you please update the pull request summary to include this information? This will help to ensure that the versioning of the package is managed properly.

grzesir added 3 commits June 29, 2024 16:42

made polygon_has_paid_subscription automatic and added polygon promot…

59451fe

…ional messages

colored the warning

dc15e18

version bump

34090c0

grzesir had a problem deploying to unit-tests June 29, 2024 20:52 — with GitHub Actions Failure

korbit-ai bot added the korbit-code-analysis label Jun 29, 2024

korbit-ai bot reviewed Jun 29, 2024

View reviewed changes

fixed test

1e9b851

grzesir temporarily deployed to unit-tests June 30, 2024 01:28 — with GitHub Actions Inactive

grzesir merged commit 0b66802 into dev Jun 30, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto polygon has paid #476

Auto polygon has paid #476

grzesir commented Jun 29, 2024

korbit-ai bot commented Jun 29, 2024

korbit-ai bot left a comment

korbit-ai bot Jun 29, 2024

korbit-ai bot Jun 29, 2024

korbit-ai bot Jun 29, 2024

korbit-ai bot Jun 29, 2024

korbit-ai bot Jun 29, 2024

korbit-ai bot Jun 29, 2024

korbit-ai bot Jun 29, 2024

korbit-ai bot Jun 29, 2024

korbit-ai bot Jun 29, 2024

korbit-ai bot Jun 29, 2024

korbit-ai bot Jun 29, 2024

korbit-ai bot Jun 29, 2024

Auto polygon has paid #476

Auto polygon has paid #476

Conversation

grzesir commented Jun 29, 2024

korbit-ai bot commented Jun 29, 2024

korbit-ai bot left a comment

Choose a reason for hiding this comment

korbit-ai bot Jun 29, 2024

Choose a reason for hiding this comment

korbit-ai bot Jun 29, 2024

Choose a reason for hiding this comment

korbit-ai bot Jun 29, 2024

Choose a reason for hiding this comment

korbit-ai bot Jun 29, 2024

Choose a reason for hiding this comment

korbit-ai bot Jun 29, 2024

Choose a reason for hiding this comment

korbit-ai bot Jun 29, 2024

Choose a reason for hiding this comment

korbit-ai bot Jun 29, 2024

Choose a reason for hiding this comment

korbit-ai bot Jun 29, 2024

Choose a reason for hiding this comment

korbit-ai bot Jun 29, 2024

Choose a reason for hiding this comment

korbit-ai bot Jun 29, 2024

Choose a reason for hiding this comment

korbit-ai bot Jun 29, 2024

Choose a reason for hiding this comment

korbit-ai bot Jun 29, 2024

Choose a reason for hiding this comment