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

Implement submit action #33

Merged

Conversation

madeddie
Copy link
Owner

@madeddie madeddie commented May 14, 2024

implements: #27
Needs quite some refactoring, but it's functional.
Needs tests too.

Summary by CodeRabbit

  • New Features

    • Introduced a new submit command for submitting tasks with options to specify filename and task ID.
  • Enhancements

    • Task details now display the submission file name.
    • Improved task parsing to include submission links.
  • Refactor

    • Streamlined task submission logic for better clarity and maintainability.

@madeddie madeddie linked an issue May 14, 2024 that may be closed by this pull request
Copy link
Contributor

coderabbitai bot commented May 14, 2024

Walkthrough

The changes bring in a new subparser for task submission, refactor task parsing to include submission links, and streamline test functions for efficiency. The submit_task function is now handled within the main function, enhancing task submission logic. These updates improve usability and code readability.

Changes

File Change Summary
moocfi_cses.py, tyora.py Added sleep import, new submit subparser, updated Task class, enhanced parse_task, and moved submit_task logic to main.
tests/test_tyora.py Simplified test functions and condensed definitions.

🐇 In the code where tasks align,
We added links to make it fine.
With submit we now can send,
Our code to tasks, from end to end.
Tests are neat, concise and bright,
Making sure our code's just right.
Hoppity hop, the rabbit's cheer,
For changes made, let's all revere! 🌟


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@senior-dev-bot senior-dev-bot bot left a comment

Choose a reason for hiding this comment

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

Feedback from Senior Dev Bot

moocfi_cses.py Outdated
Comment on lines 6 to 12
import json
from urllib.parse import urljoin
from pathlib import Path
from time import sleep
from typing import AnyStr, Optional
from xml.etree.ElementTree import Element, tostring

Choose a reason for hiding this comment

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

CODE REVIEW

The changes introduce the sleep function from the time module. Ensure that using sleep in your code is necessary, as it can introduce delays that might not be desirable, especially in IO-bound or async applications. If its use is unavoidable for rate-limiting or similar purposes, consider alternatives like async sleep functionalities in asynchronous environments or other throttling mechanisms.

No further improvements or issues are noticeable based solely on the import statements. However, always consider the impact of adding new dependencies or modules on the application's performance and maintainability.

moocfi_cses.py Outdated
Comment on lines 321 to 333
desc_div = desc_div_element if desc_div_element is not None else Element("div")
description = html2text(tostring(desc_div).decode("utf8"))
code = root.findtext(".//pre", "N/A")
submit_link_element = root.find('.//a[.="Submit"]')
submit_link = (
submit_link_element.get("href", "N/A")
if submit_link_element is not None
else "N/A"
)

submit_file = next(
iter(
[

Choose a reason for hiding this comment

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

CODE REVIEW

The changes generally look good, but you could improve readability and consistency by adopting the same pattern as your initial condition with desc_div_element. Consider using a ternary operator immediately, making it concise and consistent with the existing code style.

submit_link_element = root.find('.//a[.="Submit"]')
submit_link = submit_link_element.get("href", "N/A") if submit_link_element is not None else "N/A"

moocfi_cses.py Outdated
Comment on lines 146 to 157
parser_show.set_defaults(cmd="show")
parser_show.add_argument("task_id", help="Numerical task identifier")

# submit exercise solution subparser
parser_submit = subparsers.add_parser("submit", help="Submit an exercise solution")
parser_submit.set_defaults(cmd="submit")
parser_submit.add_argument("--filename", help="Filename of the solution to submit")
parser_submit.add_argument("task_id", help="Numerical task identifier")

return parser.parse_args(args)


Choose a reason for hiding this comment

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

CODE REVIEW

It's generally clean and modular. However, specifying the task_id as a positional argument in both subparsers could imply inconsistency if these actions (show and submit) fundamentally operate on tasks in different ways. If both actions require task_id similarly, it's alright. Otherwise, consider adding more explicit argument types for clarity, e.g., using type=int to enforce numerical IDs. Additionally, adding required=True to required arguments enhances usability.

Example:

parser_submit.add_argument("--filename", required=True, help="Filename of the solution to submit")
parser_submit.add_argument("task_id", type=int, help="Numerical task identifier")

moocfi_cses.py Outdated
Comment on lines 258 to 267
id: str
name: str
state: TaskState
description: str = "N/A"
code: str = "N/A"
submit_file: str = "N/A"
description: Optional[str] = None
code: Optional[str] = None
submit_file: Optional[str] = None
submit_link: Optional[str] = None


def parse_task_list(html: AnyStr) -> list[Task]:

Choose a reason for hiding this comment

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

CODE REVIEW

The change from default string values to Optional[str] with defaults of None is a good approach for clarifying potentially unset values. However, it's essential to consider how this affects other parts of the code that may expect string values. For instance, if there are operations like string concatenation or formatting with these variables, those would now potentially raise exceptions if not checked for None.

# Original
description: str = "N/A"

# Updated
description: Optional[str] = None

Ensure any function handling Task objects is prepared to handle these None values appropriately.

moocfi_cses.py Outdated
Comment on lines 228 to 234
return link_data


def parse_form(html: AnyStr, xpath: str = ".//form") -> dict[str, str | None]:
def parse_form(html: AnyStr, xpath: str = ".//form") -> dict:
"""Search for the first form in html and return dict with action and all other found inputs"""
form_element = htmlement.fromstring(html).find(xpath)
form_data = dict()

Choose a reason for hiding this comment

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

CODE REVIEW

It's generally good practice to be explicit about types in Python, especially with type hinting in Python 3. It improves code readability and helps with debugging and maintaining. The change here removes the specificity of the return type for parse_form, making it less informative. Consider keeping or even expanding the type hints for better clarity and utility.

Before:

def parse_form(html: AnyStr, xpath: str = ".//form") -> dict:

After:

from typing import Union, Optional, Dict

def parse_form(html: AnyStr, xpath: str = ".//form") -> Dict[str, Optional[str]]:

moocfi_cses.py Outdated
Comment on lines 345 to 351
description=description.strip(),
code=code,
submit_file=submit_file,
submit_link=submit_link,
)

return task

Choose a reason for hiding this comment

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

CODE REVIEW

The code change looks concise, adding a submit_link parameter. Ensure that:

  1. submit_link is sanitized & validated (if it's user input).
  2. Check if any default value or null handling is needed for submit_link.
  3. Update relevant function documentation or comments to include submit_link.

Example for validation (assuming submit_link is a URL):

from urllib.parse import urlparse

def is_valid_url(url):
    try:
        result = urlparse(url)
        return all([result.scheme, result.netloc])
    except ValueError:
        return False

if not is_valid_url(submit_link):
    raise ValueError("Invalid submit link")

moocfi_cses.py Outdated
Comment on lines 418 to 468
task = parse_task(html)
print_task(task)

if args.cmd == "submit":
html = session.http_request(urljoin(base_url, f"task/{args.task_id}"))
task = parse_task(html)
if not task.submit_file or task.submit_file == "N/A":
raise ValueError("No submission filename found")
if not task.submit_link or task.submit_link == "N/A":
raise ValueError("No submission link found")

submit_form_html = session.http_request(urljoin(base_url, task.submit_link))
submit_form_data = parse_form(submit_form_html)
action = submit_form_data.pop("_action")

for key, value in submit_form_data.items():
submit_form_data[key] = (None, value)
submit_form_data["file"] = (task.submit_file, open(task.submit_file, "rb"))
submit_form_data["lang"] = (None, "Python3")
submit_form_data["option"] = (None, "CPython3")

res = session.http_session.post(
urljoin(base_url, action), files=submit_form_data
)
html = res.text
result_url = res.url
print("Waiting for test results.", end="")
while "Test report" not in html:
print(".", end="")
sleep(1)
html = session.http_request(result_url)

print()
root = htmlement.fromstring(html)
submit_status_element = root.find('.//td[.="Status:"]/..') or Element("td")
submit_status_span_element = submit_status_element.find("td/span") or Element(
"span"
)
submit_status = submit_status_span_element.text or ""
submit_result_element = root.find('.//td[.="Result:"]/..') or Element("td")
submit_result_span_element = submit_result_element.find("td/span") or Element(
"span"
)
submit_result = submit_result_span_element.text or ""

print(f"Submission status: {submit_status.lower()}")
print(f"Submission result: {submit_result.lower()}")


if __name__ == "__main__":
main()

Choose a reason for hiding this comment

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

CODE REVIEW

Consider abstracting the new functionality into separate functions to improve readability and maintainability. Additionally, verifying file existence before attempting to open it improves robustness. Use with statement for file handling to ensure proper closure of the file. Refrain from directly printing dots in a loop as it's not efficient for user feedback in all environments; consider logging or other progress indication methods.

Example adjustments:

-        submit_form_data["file"] = (task.submit_file, open(task.submit_file, "rb"))
+        file_path = task.submit_file
+        if not os.path.exists(file_path):
+            raise FileNotFoundError(f"{file_path} does not exist")
+        with open(file_path, "rb") as file:
+            submit_form_data["file"] = (task.submit_file, file)
+            # Rest of file handling

Split large blocks into functions:

+    if args.cmd == "submit":
+        submit_task(args, session)

Refactoring this way enhances code structure, making it easier to manage and understand.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@senior-dev-bot can you suggests some unit tests for this method?

moocfi_cses.py Outdated
Comment on lines 357 to 369
print(f"\nSubmission file name: {task.submit_file}")


def submit_task(task_id: str, filename: str) -> None:
"""submit file to the submit form or task_id"""
# NOTE: use parse_form
...
# def submit_task(task_id: str, filename: str) -> None:
# """submit file to the submit form or task_id"""
# html = session.http_request(urljoin(base_url, f"task/{task_id}"))
# task = parse_task(html)
# answer = input("Do you want to submit this task? (y/n): ")
# if answer in ('y', 'Y'):
# with open(filename, 'r') as f:


def main() -> None:

Choose a reason for hiding this comment

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

CODE REVIEW

  • Comments on Code Removal: Commenting out submit_task without removing it or replacing it implies uncertainty about its necessity. If the function is obsolete, delete it. If its functionality is being replaced or updated, ensure the new or updated code is included and clearly documented.

    # Consider removing or properly replacing this with updated functionality.
  • Incomplete Replacement Implementation: The snippet shows the start of a replacement for submit_task, but it's incomplete and inactive due to being commented. If this is a work in progress, finish the implementation, including error handling, actual submission logic, and feedback to the user on success or failure.

    def submit_task(task_id: str, filename: str) -> None:
        """Submit file to the task."""
        # Implementation goes here...
        # Ensure error handling, user feedback, and complete functionality.
  • Code Organization: Keep your code clean and organized. If submit_task is under modification, ensure it reflects the complete and final intent. Temporary or WIP code should be clearly marked or, preferably, kept out of the main branch until complete.

    # TODO: Complete this function before merging.
  • Functionality and User Input: If replacing submit_task, ensure you're including all necessary parts such as making the actual HTTP request to submit the task, handling potential errors, and validating the user's input more robustly.

    answer = input("Do you want to submit this task? (y/n): ").strip().lower()
    if answer == 'y':
        # Proceed with submission.
    else:
        # Handle 'no' response appropriately.

Remember, keeping your codebase clean and only merging complete, fully vetted features helps maintain overall code quality and reduces technical debt.

Copy link

============================= test session starts ==============================
platform linux -- Python 3.10.14, pytest-8.2.0, pluggy-1.5.0
rootdir: /home/runner/work/moocfi_cses/moocfi_cses
plugins: requests-mock-1.12.1, cov-5.0.0
collected 11 items

tests/test_moocfi_cses.py ...........                                    [100%]

---------- coverage: platform linux, python 3.10.14-final-0 ----------
Name                        Stmts   Miss  Cover
-----------------------------------------------
moocfi_cses.py                258    152    41%
tests/__init__.py               0      0   100%
tests/test_moocfi_cses.py      48      0   100%
-----------------------------------------------
TOTAL                         306    152    50%


============================== 11 passed in 0.10s ==============================

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 63f2c3d and ee40870.
Files selected for processing (1)
  • moocfi_cses.py (8 hunks)
Additional comments not posted (6)
moocfi_cses.py (6)

9-9: Import statement for sleep is correctly added.


149-153: The new subparser for submitting tasks is correctly defined.


261-264: The changes to the Task class attributes are appropriate and align with the new functionality.


264-264: The addition of the submit_link attribute is correctly implemented.


324-330: The enhancement to extract submit_link in the parse_task function is correctly implemented.


348-348: The update to display the submission file name in the print_task function is correctly implemented.

moocfi_cses.py Outdated
Comment on lines 360 to 366
# def submit_task(task_id: str, filename: str) -> None:
# """submit file to the submit form or task_id"""
# html = session.http_request(urljoin(base_url, f"task/{task_id}"))
# task = parse_task(html)
# answer = input("Do you want to submit this task? (y/n): ")
# if answer in ('y', 'Y'):
# with open(filename, 'r') as f:
Copy link
Contributor

Choose a reason for hiding this comment

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

The submit_task function is commented out, and its logic is moved to the main function. Consider refactoring the submission logic into a separate function for better maintainability.

@madeddie madeddie force-pushed the 27-implement-exercise-solution-submission-submit-subcommand branch from ee40870 to 3e38a5a Compare May 14, 2024 14:39
Copy link

============================= test session starts ==============================
platform linux -- Python 3.10.14, pytest-8.2.0, pluggy-1.5.0
rootdir: /home/runner/work/moocfi_cses/moocfi_cses
plugins: requests-mock-1.12.1, cov-5.0.0
collected 11 items

tests/test_moocfi_cses.py ...........                                    [100%]

---------- coverage: platform linux, python 3.10.14-final-0 ----------
Name                        Stmts   Miss  Cover
-----------------------------------------------
moocfi_cses.py                258    152    41%
tests/__init__.py               0      0   100%
tests/test_moocfi_cses.py      48      0   100%
-----------------------------------------------
TOTAL                         306    152    50%


============================== 11 passed in 0.10s ==============================

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between ee40870 and 3e38a5a.
Files selected for processing (1)
  • moocfi_cses.py (8 hunks)

moocfi_cses.py Outdated
Comment on lines 149 to 153
# submit exercise solution subparser
parser_submit = subparsers.add_parser("submit", help="Submit an exercise solution")
parser_submit.set_defaults(cmd="submit")
parser_submit.add_argument("--filename", help="Filename of the solution to submit")
parser_submit.add_argument("task_id", help="Numerical task identifier")
Copy link
Contributor

Choose a reason for hiding this comment

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

Specify required=True for the --filename argument to enhance usability.

parser_submit.add_argument("--filename", required=True, help="Filename of the solution to submit")

moocfi_cses.py Outdated
@@ -221,7 +228,7 @@ def find_link(html: AnyStr, xpath: str) -> dict[str, str | None]:
return link_data


def parse_form(html: AnyStr, xpath: str = ".//form") -> dict[str, str | None]:
def parse_form(html: AnyStr, xpath: str = ".//form") -> dict:
Copy link
Contributor

Choose a reason for hiding this comment

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

Specify the value types in the return type annotation for clarity and type safety.

def parse_form(html: AnyStr, xpath: str = ".//form") -> dict[str, Optional[str]]:

moocfi_cses.py Outdated
Comment on lines 421 to 465
if args.cmd == "submit":
html = session.http_request(urljoin(base_url, f"task/{args.task_id}"))
task = parse_task(html)
if not task.submit_file or task.submit_file == "N/A":
raise ValueError("No submission filename found")
if not task.submit_link or task.submit_link == "N/A":
raise ValueError("No submission link found")

submit_form_html = session.http_request(urljoin(base_url, task.submit_link))
submit_form_data = parse_form(submit_form_html)
action = submit_form_data.pop("_action")

for key, value in submit_form_data.items():
submit_form_data[key] = (None, value)
submit_form_data["file"] = (task.submit_file, open(task.submit_file, "rb"))
submit_form_data["lang"] = (None, "Python3")
submit_form_data["option"] = (None, "CPython3")

res = session.http_session.post(
urljoin(base_url, action), files=submit_form_data
)
html = res.text
result_url = res.url
print("Waiting for test results.", end="")
while "Test report" not in html:
print(".", end="")
sleep(1)
html = session.http_request(result_url)

print()
root = htmlement.fromstring(html)
submit_status_element = root.find('.//td[.="Status:"]/..') or Element("td")
submit_status_span_element = submit_status_element.find("td/span") or Element(
"span"
)
submit_status = submit_status_span_element.text or ""
submit_result_element = root.find('.//td[.="Result:"]/..') or Element("td")
submit_result_span_element = submit_result_element.find("td/span") or Element(
"span"
)
submit_result = submit_result_span_element.text or ""

print(f"Submission status: {submit_status.lower()}")
print(f"Submission result: {submit_result.lower()}")

Copy link
Contributor

Choose a reason for hiding this comment

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

Consider refactoring the submission logic into a separate function for better readability and maintainability.

def submit_task(session, base_url, task_id, filename):
    html = session.http_request(urljoin(base_url, f"task/{task_id}"))
    task = parse_task(html)
    if not task.submit_file or task.submit_file == "N/A":
        raise ValueError("No submission filename found")
    if not task.submit_link or task.submit_link == "N/A":
        raise ValueError("No submission link found")

    submit_form_html = session.http_request(urljoin(base_url, task.submit_link))
    submit_form_data = parse_form(submit_form_html)
    action = submit_form_data.pop("_action")

    for key, value in submit_form_data.items():
        submit_form_data[key] = (None, value)
    submit_form_data["file"] = (task.submit_file, open(task.submit_file, "rb"))
    submit_form_data["lang"] = (None, "Python3")
    submit_form_data["option"] = (None, "CPython3")

    res = session.http_session.post(
        urljoin(base_url, action), files=submit_form_data
    )
    html = res.text
    result_url = res.url
    print("Waiting for test results.", end="")
    while "Test report" not in html:
        print(".", end="")
        sleep(1)
        html = session.http_request(result_url)

    print()
    root = htmlement.fromstring(html)
    submit_status_element = root.find('.//td[.="Status:"]/..') or Element("td")
    submit_status_span_element = submit_status_element.find("td/span") or Element("span")
    submit_status = submit_status_span_element.text or ""
    submit_result_element = root.find('.//td[.="Result:"]/..') or Element("td")
    submit_result_span_element = submit_result_element.find("td/span") or Element("span")
    submit_result = submit_result_span_element.text or ""

    print(f"Submission status: {submit_status.lower()}")
    print(f"Submission result: {submit_result.lower()}")

# In main function
if args.cmd == "submit":
    submit_task(session, base_url, args.task_id, args.filename)

@madeddie madeddie force-pushed the 27-implement-exercise-solution-submission-submit-subcommand branch from 3e38a5a to e555cc0 Compare May 16, 2024 16:13
Copy link

============================= test session starts ==============================
platform linux -- Python 3.10.14, pytest-8.2.0, pluggy-1.5.0
rootdir: /home/runner/work/tyora/tyora
plugins: cov-5.0.0, requests-mock-1.12.1
collected 11 items

tests/test_tyora.py ...........                                          [100%]

---------- coverage: platform linux, python 3.10.14-final-0 ----------
Name                  Stmts   Miss  Cover
-----------------------------------------
tests/__init__.py         0      0   100%
tests/test_tyora.py      44      0   100%
tyora.py                258    152    41%
-----------------------------------------
TOTAL                   302    152    50%


============================== 11 passed in 0.12s ==============================

@madeddie madeddie changed the title First version of the submit task code Implement submit action May 16, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 3e38a5a and e555cc0.
Files selected for processing (2)
  • tests/test_tyora.py (2 hunks)
  • tyora.py (9 hunks)
Files skipped from review due to trivial changes (1)
  • tests/test_tyora.py
Additional comments not posted (6)
tyora.py (6)

9-9: Import statement for sleep looks good.


149-153: Subparser for submitting tasks looks good.


261-264: Addition of submit_link attribute in Task class looks good.


326-331: Changes to parse_task function to extract and assign submit_link look good.

Also applies to: 350-350


362-368: Commenting out the submit_task function is acceptable.


423-467: Changes to the main function to handle task submissions look good.

…mand' of github.com:madeddie/moocfi_cses into 27-implement-exercise-solution-submission-submit-subcommand
Copy link

============================= test session starts ==============================
platform linux -- Python 3.10.14, pytest-8.2.0, pluggy-1.5.0
rootdir: /home/runner/work/tyora/tyora
plugins: cov-5.0.0, requests-mock-1.12.1
collected 11 items

tests/test_tyora.py ...........                                          [100%]

---------- coverage: platform linux, python 3.10.14-final-0 ----------
Name                  Stmts   Miss  Cover
-----------------------------------------
tests/__init__.py         0      0   100%
tests/test_tyora.py      44      0   100%
tyora.py                258    152    41%
-----------------------------------------
TOTAL                   302    152    50%


============================== 11 passed in 0.10s ==============================

Copy link

============================= test session starts ==============================
platform linux -- Python 3.10.14, pytest-8.2.0, pluggy-1.5.0
rootdir: /home/runner/work/tyora/tyora
plugins: cov-5.0.0, requests-mock-1.12.1
collected 11 items

tests/test_tyora.py ...........                                          [100%]

---------- coverage: platform linux, python 3.10.14-final-0 ----------
Name                  Stmts   Miss  Cover
-----------------------------------------
tests/__init__.py         0      0   100%
tests/test_tyora.py      44      0   100%
tyora.py                267    161    40%
-----------------------------------------
TOTAL                   311    161    48%


============================== 11 passed in 0.13s ==============================

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between e555cc0 and b2af18f.
Files selected for processing (1)
  • tyora.py (10 hunks)
Additional comments not posted (7)
tyora.py (7)

9-9: Import of sleep from the time module is appropriate for the new submission functionality where delays are expected.


149-157: The addition of a new subparser for submitting exercise solutions is well-implemented. The arguments are clear and the help messages are informative.


264-267: The addition of optional attributes (description, code, submit_file, submit_link) to the Task class is a good use of Python's Optional type hint to handle potentially missing data. This change enhances the flexibility of the Task data structure.


293-297: The logic to determine the task state based on the class attribute in the HTML element is clear and concise.


321-325: Proper error handling in the parse_task function when task ID or name is missing. This will prevent the application from proceeding with incomplete data.


332-339: The extraction of submit_link from the HTML is crucial for the submission functionality. Ensure that the link is correctly parsed and handled in subsequent steps.


357-357: The construction of the Task object with all the newly added optional fields is correctly implemented.

@@ -221,7 +231,7 @@ def find_link(html: AnyStr, xpath: str) -> dict[str, str | None]:
return link_data


def parse_form(html: AnyStr, xpath: str = ".//form") -> dict[str, str | None]:
def parse_form(html: AnyStr, xpath: str = ".//form") -> dict:
Copy link
Contributor

Choose a reason for hiding this comment

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

The modification in the parse_form function to remove type hints for return values aligns with the changes in other parts of the codebase. However, consider adding a more detailed docstring to explain the parameters and the return type more clearly.

def parse_form(html: AnyStr, xpath: str = ".//form") -> dict:
    """
    Search for the first form in html and return a dictionary with action and all other found inputs.

    Args:
        html (AnyStr): HTML content as a string.
        xpath (str): XPath query to locate the form.

    Returns:
        dict: Dictionary containing form action and inputs.
    """
    # function body remains the same

tyora.py Outdated
Comment on lines 434 to 479
if args.cmd == "submit":
html = session.http_request(urljoin(base_url, f"task/{args.task_id}"))
task = parse_task(html)
if not task.submit_file and not args.filename:
raise ValueError("No submission filename found")
if not task.submit_link:
raise ValueError("No submission link found")
submit_file = args.filename or task.submit_file or ""

submit_form_html = session.http_request(urljoin(base_url, task.submit_link))
submit_form_data = parse_form(submit_form_html)
action = submit_form_data.pop("_action")

for key, value in submit_form_data.items():
submit_form_data[key] = (None, value)
submit_form_data["file"] = (submit_file, open(submit_file, "rb"))
submit_form_data["lang"] = (None, "Python3")
submit_form_data["option"] = (None, "CPython3")

res = session.http_session.post(
urljoin(base_url, action), files=submit_form_data
)
html = res.text
result_url = res.url
print("Waiting for test results.", end="")
while "Test report" not in html:
print(".", end="")
sleep(1)
html = session.http_request(result_url)

print()
root = htmlement.fromstring(html)
submit_status_element = root.find('.//td[.="Status:"]/..') or Element("td")
submit_status_span_element = submit_status_element.find("td/span") or Element(
"span"
)
submit_status = submit_status_span_element.text or ""
submit_result_element = root.find('.//td[.="Result:"]/..') or Element("td")
submit_result_span_element = submit_result_element.find("td/span") or Element(
"span"
)
submit_result = submit_result_span_element.text or ""

print(f"Submission status: {submit_status.lower()}")
print(f"Submission result: {submit_result.lower()}")

Copy link
Contributor

Choose a reason for hiding this comment

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

The submission logic integrated into the main function is complex and involves multiple steps, including form parsing, file submission, and status checking. Consider refactoring this into a separate function or class to improve maintainability and testability.

def submit_task(session: Session, task: Task, filename: str) -> None:
    """
    Handles the submission of a task using the provided session and task details.

    Args:
        session (Session): The session object with login details and cookies.
        task (Task): The task object containing submission details.
        filename (str): The filename of the solution to submit.
    """
    # Move the submission logic here

Copy link

============================= test session starts ==============================
platform linux -- Python 3.10.14, pytest-8.2.0, pluggy-1.5.0
rootdir: /home/runner/work/tyora/tyora
plugins: cov-5.0.0, requests-mock-1.12.1
collected 11 items

tests/test_tyora.py ...........                                          [100%]

---------- coverage: platform linux, python 3.10.14-final-0 ----------
Name                  Stmts   Miss  Cover
-----------------------------------------
tests/__init__.py         0      0   100%
tests/test_tyora.py      44      0   100%
tyora.py                270    163    40%
-----------------------------------------
TOTAL                   314    163    48%


============================== 11 passed in 0.10s ==============================

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between b2af18f and bd13201.
Files selected for processing (1)
  • tyora.py (10 hunks)
Additional comments not posted (9)
tyora.py (9)

9-9: Added import of sleep from the time module.

This import is necessary for the new submission logic which includes a delay loop.


149-156: Added a new subparser for submitting exercise solutions.

This change aligns with the PR's objective to implement a submit action. The arguments and help texts are clear and appropriate.


234-234: Modification to parse_form function to remove type hints for return values.

The previous review comment suggested adding a detailed docstring, which has been implemented. This enhances clarity and maintainability.


264-267: Updated Task class to include optional attributes.

The addition of submit_link and making other attributes optional supports the new functionality for task submission and aligns with the changes in the system's requirements.


293-297: Logic to determine task state based on class attributes in HTML.

This implementation is efficient and clear, using conditional expressions to set the task state.


321-325: Error handling in parse_task function.

Proper error handling for missing task ID and name ensures robustness and user-friendly error messages.


332-338: Extraction of submit_link from HTML in parse_task.

This is crucial for the submission process, ensuring that the link is dynamically retrieved from the task details.


369-374: Commented out submit_task function.

As per the PR description, the submission logic has been moved to the main function. This centralizes the submission logic, potentially simplifying maintenance.


453-487: Integrated submission logic into the main function.

This integration is complex but necessary for the new functionality. However, consider refactoring this into a separate function or class to improve maintainability and testability, as previously suggested.

@madeddie madeddie merged commit 67f9a42 into main May 20, 2024
1 check passed
@madeddie madeddie deleted the 27-implement-exercise-solution-submission-submit-subcommand branch May 20, 2024 01:28
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.

Implement exercise solution submission (submit) subcommand
1 participant