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

Beta #255

Open
wants to merge 91 commits into
base: master
Choose a base branch
from
Open

Beta #255

wants to merge 91 commits into from

Conversation

jammesop007aha
Copy link

No description provided.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @jammesop007aha - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 8 issues found
  • 🟢 Security: all looks good
  • 🟡 Testing: 3 issues found
  • 🟡 Complexity: 2 issues found

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

from gdown import GDriveFileTransferError
from gdown.download import DownloadError
from gdown.gdrive import GDriveFile
from gdown.service import ServiceException
Copy link

Choose a reason for hiding this comment

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

issue (code_refinement): Repeated import statements detected.

The import statement for 'ServiceException' from 'gdown.service' is repeated multiple times. Consider importing it once to maintain code cleanliness and avoid redundancy.

"""
try:
return self.processed_raw() / (time() - self.__start_time)
except ZeroDivisionError:
Copy link

Choose a reason for hiding this comment

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

suggestion (code_refinement): Handle potential division by zero in eta method.

Consider adding a specific check for self.speed_raw() being zero before performing the division to prevent ZeroDivisionError.

Suggested change
except ZeroDivisionError:
if self.speed_raw() == 0:
return "ETA: Not available"
else:
eta_seconds = self.size_raw() / self.speed_raw()
return f"ETA: {self.format_time(eta_seconds)}"

LOGGER.info(f'Cancelling Archive: {self.__name}')
if self.__listener.suproc is not None:
self.__listener.suproc.kill()
else:
self.__listener.suproc = 'cancelled'
await self.__listener.onUploadError('archiving stopped by user!')
await self.__listener.on_upload_error('archiving stopped by user!')
Copy link

Choose a reason for hiding this comment

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

suggestion (code_clarification): Use consistent terminology in user messages.

The term 'archiving stopped by user' could be aligned with other similar messages in the system for consistency. Consider using 'cancelled by user' if that is the standard terminology used elsewhere.

Suggested change
await self.__listener.on_upload_error('archiving stopped by user!')
await self.__listener.on_upload_error('cancelled by user!')

Comment on lines -7 to -10
self.__size = size
self.__gid = gid
self.upload_details = upload_details
self.message = message
Copy link

Choose a reason for hiding this comment

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

suggestion (code_refinement): Consider reordering the initialization of attributes for clarity.

Initializing attributes in a consistent order can improve code readability. Consider grouping similar attributes together or ordering them logically.

Suggested change
self.__size = size
self.__gid = gid
self.upload_details = upload_details
self.message = message
class DDLStatus:
def __init__(self, obj, size, message, gid, upload_details):
self.__obj = obj
self.__size = size
self.__gid = gid
self.upload_details = upload_details


def __init__(self, listener):
def __init__(self, listener: Any):
Copy link

Choose a reason for hiding this comment

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

suggestion (code_refinement): Consider using a more specific type than Any for listener.

Using a more specific type can enhance type checking and make the codebase more robust and maintainable.

Suggested change
def __init__(self, listener: Any):
def __init__(self, listener: ListenerType):


:param message: The message object containing the file.
:param path: The path to save the file.
:return: Whether the download was successful.
Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Return type in docstring should be more specific

The return type in the docstring for __download method should specify that it returns a boolean. This makes it clearer what the expected type of the return value should be.

Suggested change
:return: Whether the download was successful.
:return: bool - True if the download was successful, False otherwise.

elif not self.__is_cancelled:
await self.__onDownloadError('Internal Error occurred')
return False
Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Ensure consistency in error handling

It appears that the method __onDownloadError consistently returns False. Ensure that this behavior is consistent across all error handling in the class to maintain predictable behavior.

@@ -1,86 +1,105 @@
from asyncio import create_subprocess_exec, gather
import asyncio
Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Consider grouping standard library imports.

It's a common practice to group all standard library imports together for better readability and maintainability.

Suggested change
import asyncio
import asyncio
import os
import sys
from time import time, monotonic

@@ -347,10 +357,8 @@ async def __run_multi():
pssw = args['-p'] or args['-pass']
if ussr or pssw:
auth = f"{ussr}:{pssw}"
auth = "Basic " + b64encode(auth.encode()).decode('ascii')
Copy link

Choose a reason for hiding this comment

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

issue (complexity): Consider simplifying the error handling and maintaining detailed error messages for better debuggability.

The refactoring introduces several changes that could potentially increase the complexity and reduce maintainability. Here are some key observations:

  1. Error Handling and Code Clarity:

    • The addition of a try-except block around the conversion of args['-i'] to an integer adds robustness by handling potential exceptions. However, it also introduces additional branching and error handling that wasn't present before.
    • The removal of detailed error messages and traceback logging (format_exc()) in favor of more generic error messages could make debugging more difficult.
  2. Code Structure and Readability:

    • The removal of several condition checks and the simplification of some expressions could be seen as a reduction in complexity. However, it also removes certain functionalities like handling specific user credentials (ussr and pssw) which were present in the original code.
    • The restructuring of conditions and the handling of different link types (e.g., Telegram links, direct links) changes the flow and logic significantly, potentially making it harder to follow.
  3. Functionality Changes:

    • The handling of headers and authentication has been modified. In the original code, headers are built dynamically based on conditions, whereas in the new code, headers are set in a more static and less flexible manner.
    • The new code seems to simplify some of the operations by removing options and checks (e.g., removing the decryption and screenshot handling). This might reduce the feature set provided by the code.

While the new code introduces some simplifications, it also increases complexity in other areas by changing the flow and potentially reducing the clarity and debuggability of the code. Consider balancing these aspects to enhance both maintainability and functionality.

@@ -1,9 +1,16 @@
#!/usr/bin/env python3
from speedtest import Speedtest
import asyncio
Copy link

Choose a reason for hiding this comment

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

issue (complexity): Consider simplifying the new code additions to enhance maintainability.

The refactored code introduces significantly more complexity compared to the original. Here are a few points to consider:

  1. Additional Dependencies: The introduction of new libraries like aiohttp and PIL increases the dependency management overhead. It's crucial to ensure these are well-integrated and maintained throughout the project lifecycle.

  2. Error Handling and Asynchronous Operations: The new asynchronous operations and error handling mechanisms add layers of complexity. While these are necessary for the new functionality, ensure that they are robust and well-documented to aid future maintenance.

  3. Image Processing: The steps involved in downloading, processing, and cleaning up the image files introduce multiple potential points of failure. Consider encapsulating these steps into dedicated functions or classes to improve modularity and testability.

  4. Code Readability: The increased length and nested structures in the code can impact readability and maintainability. Simplifying these, possibly by breaking down complex functions into smaller, more manageable ones, could enhance understandability and reduce the cognitive load for future developers.

By addressing these points, the code can be made more maintainable and robust, aligning better with best practices in software development.

else:
await query.answer(url=f"https://t.me/{bot_name}?start=start")
raise ValueError("Save mode not enabled.")
except (StopIteration, KeyError) as e:

Choose a reason for hiding this comment

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

ℹ️ Codacy found a minor Code Style issue: local variable 'e' is assigned to but never used (F841)

The issue identified by the Prospector linter is that the variable e is assigned the exception caught by the except block but is never used within that block. This is considered poor practice because it creates an unnecessary variable, which can make the code less readable and potentially hide bugs.

To fix the issue, we can simply remove the assignment of the exception to the variable e since it's not being used. Here's the suggested single line change:

Suggested change
except (StopIteration, KeyError) as e:
except (StopIteration, KeyError):

This comment was generated by an experimental AI tool.

repo = upstream_repo.split("/")
upstream_repo = "https://github.com/{}".format("/".join(repo[-2:]))

log_info("Successfully updated with latest commits !!")

Choose a reason for hiding this comment

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

ℹ️ Codacy found a minor Code Style issue: undefined name 'log_info' (F821)

The issue identified by Prospector with the code style message "undefined name 'log_info' (F821)" indicates that the function log_info is being called, but it has not been defined or imported in the snippet provided. This means that the Python interpreter does not know what log_info refers to, which will result in a NameError at runtime.

To fix this issue, you would need to ensure that log_info is either defined or imported correctly in your code. If log_info is a custom logging function that you have defined elsewhere in your codebase, you need to import it. If it's part of a module, you need to import the module and use the function correctly.

Since I don't have the full context of where log_info is supposed to come from, I'll provide a generic fix by importing a hypothetical log_info function from a module named logger. If log_info is defined in a different module or has a different origin, you'll need to adjust the import statement accordingly.

Here's the suggested code change:

Suggested change
log_info("Successfully updated with latest commits !!")
from logger import log_info

This comment was generated by an experimental AI tool.

@@ -1,4 +1,5 @@
#!/usr/bin/env python3
from typing import Any, Callable, Coroutine, Dict, Set, Union

Choose a reason for hiding this comment

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

⚠️ Codacy found a medium Code Style issue: Unused Callable imported from typing

The issue identified by Pylint is that the Callable class from the typing module is imported but not used anywhere in the code. This is a common issue that occurs when a developer imports more than what is actually needed for the script to function. Having unused imports can lead to confusion and less readable code, as well as slightly increased loading times.

To fix this issue, you should remove the unused Callable import from the typing module. Here's the suggested change:

Suggested change
from typing import Any, Callable, Coroutine, Dict, Set, Union
from typing import Any, Coroutine, Dict, Set, Union

This comment was generated by an experimental AI tool.

import os
import sys
import asyncio
import json

Choose a reason for hiding this comment

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

ℹ️ Codacy found a minor Code Style issue: 'json' imported but unused (F401)

The issue identified by the Prospector linter is that the json module is imported but not used anywhere in the code fragment provided. This is considered a Code Style issue because it's generally good practice to avoid importing modules that are not being used, as it can lead to unnecessary use of memory and can make the code less readable.

To fix this issue, you should remove the import statement for the json module. Here's the code suggestion:

Suggested change
import json
# Remove the unused import

This comment was generated by an experimental AI tool.

import os
import asyncio
import pathlib as plib
from typing import List, Tuple, Union

Choose a reason for hiding this comment

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

⚠️ Codacy found a medium Code Style issue: Unused List imported from typing

The issue indicated by Pylint is that the List class from the typing module is imported but not used anywhere in the code fragment you've provided. This is a common issue when refactoring code or when an import is added with the anticipation of future use that never materializes. It's good practice to remove unused imports as they can lead to confusion about code dependencies and can slightly increase the startup time of a script.

Here's the suggested change to remove the unused import:

Suggested change
from typing import List, Tuple, Union
from typing import Tuple, Union

This comment was generated by an experimental AI tool.

<b>Timeout:</b> 120s

<i>Send /stop to Stop Process</i>""")
session_dict['message'] = sess_msg
await wrap_future(invoke(client, message, 'API_ID'))
await wrap_future(invoke(message, 'API_ID'))

Choose a reason for hiding this comment

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

Codacy found a critical Code Style issue: No value for argument 'key' in function call

The issue that Pylint is pointing out indicates that the invoke function is being called without providing a required argument. It seems that the invoke function requires a 'key' argument, but in the call within the provided code, only message and 'API_ID' are provided.

Without the full context of what the invoke function signature looks like, it's hard to provide an exact fix. However, assuming that the 'API_ID' string is meant to be the value for the 'key' argument, the line should be corrected to include the 'key' as a named argument.

Here's the suggested fix:

Suggested change
await wrap_future(invoke(message, 'API_ID'))
await wrap_future(invoke(message, key='API_ID'))

This comment was generated by an experimental AI tool.


# Install any needed packages specified in requirements.txt
RUN apt-get update && apt-get install -y --no-cache-dir \

Choose a reason for hiding this comment

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

⚠️ Codacy found a medium Error Prone issue: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>

The issue identified by Hadolint is that the Dockerfile is using the apt-get install command without specifying the exact versions of the packages to be installed. This can lead to non-deterministic builds because the latest version of the packages available at the time of the build will be installed, which might change over time. To ensure a consistent and repeatable build environment, it's recommended to pin specific versions of the packages.

Here's the suggested fix for the Dockerfile line:

Suggested change
RUN apt-get update && apt-get install -y --no-cache-dir \
RUN apt-get update && apt-get install -y --no-install-recommends build-essential=<version> \

Please replace <version> with the specific version of the build-essential package that you want to install. Also, I removed the non-existent --no-cache-dir option from the apt-get install command, which is not valid for apt-get (it's a pip option). Moreover, I included --no-install-recommends to avoid installing unnecessary packages.


This comment was generated by an experimental AI tool.

@@ -1,9 +1,16 @@
#!/usr/bin/env python3
from speedtest import Speedtest
import asyncio

Choose a reason for hiding this comment

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

ℹ️ Codacy found a minor Code Style issue: 'asyncio' imported but unused (F401)

The issue identified by the Prospector linter is that the asyncio module has been imported but is not being used anywhere in the code fragment provided. This is considered a Code Style issue because unnecessary imports can cause confusion and clutter in the code, making it less readable and potentially increasing the memory footprint of the program.

To fix this issue, you should remove the unused import statement. Here's the code suggestion to address this:

Suggested change
import asyncio
# Removed the unused import statement

Make sure to check the rest of your code to ensure that asyncio is indeed not used anywhere. If it turns out that asyncio is used in parts of the code not shown in the fragment, you should not remove the import statement.


This comment was generated by an experimental AI tool.


from bot import bot, bot_cache, categories_dict, download_dict, download_dict_lock
from bot.helper.ext_utils.bot_utils import MirrorStatus, arg_parser, fetch_user_tds, fetch_user_dumps, getDownloadByGid, is_gdrive_link, new_task, sync_to_async, get_readable_time
from bot.helper.ext_utils.bot_utils import MirrorStatus, arg_parser, fetch_user_tds, fetch_user_dumps, get_download_by_gid, is_gdrive_link, new_task, sync_to_async, get_readable_time

Choose a reason for hiding this comment

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

ℹ️ Codacy found a minor Code Style issue: line too long (182 > 159 characters) (E501)

The issue identified by the Prospector linter is that the line of code exceeds the maximum recommended length of 159 characters. This can make the code harder to read and maintain, especially when viewing it in editors with limited horizontal space or when doing code reviews.

To fix this issue, we can split the import statement into multiple lines. Here's a suggestion to reformat the long import line into a multi-line import which adheres to the PEP 8 style guide:

from bot.helper.ext_utils.bot_utils import (
    MirrorStatus, arg_parser, fetch_user_tds, fetch_user_dumps, 
    get_download_by_gid, is_gdrive_link, new_task, sync_to_async, 
    get_readable_time
)

This comment was generated by an experimental AI tool.

headers = f"{header}: {value}"
return [resp[0].get("link"), headers]

page_token, turn_page = '', False

Choose a reason for hiding this comment

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

ℹ️ Codacy found a minor Code Style issue: expected 2 blank lines after class or function definition, found 1 (E305)

The issue identified by the Prospector linter is that it expects two blank lines after a class or function definition, but only one blank line is found. According to PEP 8, which is the style guide for Python code, there should be two blank lines before top-level function and class definitions.

Here's the code suggestion to fix the issue:

    return [resp[0].get("link"), headers]

This comment was generated by an experimental AI tool.

import pathlib
from typing import Any, Dict, List, Optional

import aiofiles

Choose a reason for hiding this comment

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

⚠️ Codacy found a medium Code Style issue: Unused import aiofiles

The issue identified by Pylint indicates that the aiofiles module is imported but not used anywhere in the code fragment provided. This can happen when the code has been refactored or when the initial import was added for a feature that was never implemented. Unused imports can clutter the codebase, making it harder to read and maintain, and potentially causing unnecessary increases in memory usage or startup time.

To fix this issue, you should remove the unused import statement. Here's the code suggestion to resolve the Pylint warning:

Suggested change
import aiofiles
# Remove the unused import statement

This comment was generated by an experimental AI tool.

from aiofiles.os import path as aiopath, makedirs
from aiofiles import open as aiopen
from aiorwlock import RWLock

Choose a reason for hiding this comment

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

ℹ️ Codacy found a minor Code Style issue: 'aiorwlock.RWLock' imported but unused (F401)

The issue that Prospector is reporting is that the RWLock class from the aiorwlock module has been imported but is not being used anywhere in the code. This is considered a code style issue because it's generally good practice to avoid importing unnecessary modules or classes, as it can lead to confusion and clutter in the codebase.

To resolve this issue, you should remove the import statement for RWLock if it is indeed not used anywhere in the DbManager class or elsewhere in the code. Here's the suggested change:

Suggested change
from aiorwlock import RWLock
# from aiorwlock import RWLock

This comment was generated by an experimental AI tool.

@@ -1,18 +1,22 @@
#!/usr/bin/env python3
import os
import asyncio

Choose a reason for hiding this comment

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

ℹ️ Codacy found a minor Code Style issue: 'asyncio' imported but unused (F401)

The issue identified by the Prospector linter indicates that the asyncio module is imported in the script but is not being used anywhere in the code. This is considered a code style issue because unused imports can clutter the codebase, making it harder to read and potentially leading to confusion about the code's dependencies. It is generally good practice to remove unused imports to keep the code clean and efficient.

To fix this issue, simply remove the line that imports the asyncio module. Here's the code suggestion to address the issue:

# Remove the unused import
# import asyncio

This comment was generated by an experimental AI tool.


import aiohttp
import cloudscraper

Choose a reason for hiding this comment

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

ℹ️ Codacy found a minor Code Style issue: 'cloudscraper' imported but unused (F401)

The issue reported by Prospector's linter indicates that the cloudscraper module is imported but not used anywhere in the code fragment provided. This is considered a Code Style issue because having unused imports can lead to confusion and unnecessary clutter in the codebase. It's a good practice to remove unused imports to keep the code clean and efficient.

Here's the code suggestion to fix the issue:

Suggested change
import cloudscraper
# Removed the unused import of cloudscraper

This comment was generated by an experimental AI tool.

@@ -1,293 +1,610 @@
#!/usr/bin/env python3
import os

Choose a reason for hiding this comment

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

⚠️ Codacy found a medium Code Style issue: Unused import os

The issue identified by Pylint indicates that the os module has been imported but is not used anywhere in the code fragment provided. Having unused imports is not ideal as it can lead to confusion about code dependencies and can slightly increase the memory footprint of the program.

To fix this issue, you should remove the unused import. Here's the code suggestion to address the issue:

Suggested change
import os
# Removed the unused import statement

This comment was generated by an experimental AI tool.

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.

None yet

3 participants