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

Added github workflow for automatic parsing and updated code accordingly #26

Merged
merged 8 commits into from
Jan 8, 2025

Conversation

andeplane
Copy link
Owner

@andeplane andeplane commented Jan 8, 2025

PR Type

Enhancement, Tests


Description

  • Added a GitHub workflow for automated script execution and PR creation.

  • Enhanced parse.py to handle file downloads and updated file paths.

  • Introduced a requirements.txt file for dependency management.

  • Configured automated daily and manual triggers for the workflow.


Changes walkthrough 📝

Relevant files
Enhancement
parse.py
Enhance parsing script with file handling updates               

parse_fyrlys/parse.py

  • Added logic to download the PDF if missing.
  • Updated file paths for better organization.
  • Adjusted output file paths for JSON and QML files.
  • +12/-3   
    Tests
    parse_fyrliste.yaml
    Add GitHub Actions workflow for automation                             

    .github/workflows/parse_fyrliste.yaml

  • Added a GitHub Actions workflow for automation.
  • Configured daily and manual triggers for the workflow.
  • Automated script execution and PR creation.
  • +71/-0   
    Configuration changes
    requirements.txt
    Introduce requirements file for dependencies                         

    parse_fyrlys/requirements.txt

    • Added dependencies for pdfplumber and tqdm.
    +2/-0     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    qodo-merge-pro bot commented Jan 8, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    The PDF download code lacks error handling for network issues or invalid responses. Should handle potential exceptions when downloading and writing the file.

    response = requests.get("https://nfs.kystverket.no/fyrlister/Fyrliste_HeleLandet.pdf")
    with open(pdf_path, "wb") as f:
        f.write(response.content)
    Workflow Security

    The workflow uses GITHUB_TOKEN but doesn't specify minimum required permissions. Consider explicitly defining required permissions for security.

    token: ${{ secrets.GITHUB_TOKEN }}

    Copy link
    Contributor

    qodo-merge-pro bot commented Jan 8, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add proper error handling for HTTP requests to handle network failures and invalid responses

    Add error handling for the HTTP request when downloading the PDF file. The current
    implementation doesn't handle potential network issues or invalid responses.

    parse_fyrlys/parse.py [163-165]

     response = requests.get("https://nfs.kystverket.no/fyrlister/Fyrliste_HeleLandet.pdf")
    +response.raise_for_status()
     with open(pdf_path, "wb") as f:
         f.write(response.content)
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding error handling for HTTP requests is crucial for reliability, as network failures could silently corrupt the PDF file or crash the script. The suggestion properly uses raise_for_status() to catch HTTP errors.

    8
    General
    Update deprecated GitHub Actions workflow commands to use the current recommended syntax

    The set-output command is deprecated in GitHub Actions. Use the new GITHUB_OUTPUT
    environment file approach instead.

    .github/workflows/parse_fyrliste.yaml [52-53]

    -echo "::set-output name=changes::false"
    -echo "::set-output name=changes::true"
    +echo "changes=false" >> $GITHUB_OUTPUT
    +echo "changes=true" >> $GITHUB_OUTPUT
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Using deprecated GitHub Actions syntax could lead to future compatibility issues. The suggestion updates to the current recommended approach using GITHUB_OUTPUT, which is important for maintaining workflow reliability.

    7
    Organize imports properly at the top of the file to follow Python conventions and prevent runtime issues

    The requests module is used but not imported at the top of the file. Move all
    imports to the top to follow Python best practices and prevent potential import
    errors.

    parse_fyrlys/parse.py [162]

    +# At the top with other imports:
    +import os
    +import json
    +import time
     import requests
    -response = requests.get("https://nfs.kystverket.no/fyrlister/Fyrliste_HeleLandet.pdf")
    +from tqdm import tqdm
    +import pdfplumber
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: Moving the requests import to the top of the file follows Python best practices and prevents potential import-related issues, though the current code would still work as is.

    5

    Copy link
    Contributor

    qodo-merge-pro bot commented Jan 8, 2025

    Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

    CI Failure Feedback 🧐

    Action: generate-and-pr

    Failed stage: Run Script [❌]

    Failed test name: ""

    Failure summary:

    The action failed because the required Python package requests is not installed. The script
    attempted to import the requests module at line 162 in
    /home/runner/work/FyrLysAR/FyrLysAR/parse_fyrlys/parse.py but encountered a ModuleNotFoundError.

    Relevant error logs:
    1:  ##[group]Operating System
    2:  Ubuntu
    ...
    
    572:  Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.11/x64
    573:  Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.11/x64
    574:  LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.11.11/x64/lib
    575:  ##[endgroup]
    576:  Traceback (most recent call last):
    577:  Downloading Fyrliste_HeleLandet.pdf from https://nfs.kystverket.no/fyrlister/Fyrliste_HeleLandet.pdf
    578:  File "/home/runner/work/FyrLysAR/FyrLysAR/parse_fyrlys/parse.py", line 162, in <module>
    579:  import requests
    580:  ModuleNotFoundError: No module named 'requests'
    581:  ##[error]Process completed with exit code 1.
    

    ✨ CI feedback usage guide:

    The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
    The tool analyzes the failed checks and provides several feedbacks:

    • Failed stage
    • Failed test name
    • Failure summary
    • Relevant error logs

    In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

    /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"
    

    where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

    Configuration options

    • enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
    • excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
    • enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
    • persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
    • final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

    See more information about the checks tool in the docs.

    @andeplane andeplane force-pushed the anders/github_workflow branch from 420826d to 189c0f1 Compare January 8, 2025 20:07
    @andeplane andeplane merged commit 759f57f into main Jan 8, 2025
    @andeplane andeplane deleted the anders/github_workflow branch January 8, 2025 20:20
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant