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 set recommended appimage URL and config #54

Merged
merged 5 commits into from
Feb 9, 2024

Conversation

n8marti
Copy link
Collaborator

@n8marti n8marti commented Feb 1, 2024

Partial fix for #5:

  • set recommended appimage based on latest release from FaithLife-Community repo

Bonus:

  • a couple of small fixes for dependency checking

@n8marti n8marti requested a review from thw26 February 1, 2024 14:07
Copy link
Collaborator

@thw26 thw26 left a comment

Choose a reason for hiding this comment

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

Could we turn this into a generic function for getting release info from GitHub, such that we could apply it to the wine-appimages repo and the installer repo? This way the function could supply code to both getting the recommended appimage for #5 and also for #46?

set_recommended_appimage_config() would then call this generic getter function and set the config to the latest release by running the succeeding parsing code.

utils.py Outdated
Comment on lines 986 to 991
releases_url = "https://api.github.com/repos/FaithLife-Community/wine-appimages/releases" # noqa: E501
json_data = json.loads(net_get(releases_url))
appimage_url = json_data[0].get('assets')[0].get('browser_download_url')
logging.info(f"Recommended AppImage URL: {appimage_url}")
config.RECOMMENDED_WINE64_APPIMAGE_FULL_URL = appimage_url
config.RECOMMENDED_WINE64_APPIMAGE_FULL_FILENAME = os.path.basename(appimage_url) # noqa: E501
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we turn this into a generic function for getting release info from GitHub, such that we could apply it to the wine-appimages repo and the installer repo? This way the function could supply code to both getting the recommended appimage for #5 and also for #46?

set_recommended_appimage_config() would then call this generic getter function and set the config to the latest release by running the succeeding parsing code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My thoughts then are that we could work a second function that also sets the config with the latest installer release, too. We could run this function in the background or during start to check if there is a new release for either.

@thw26 thw26 mentioned this pull request Feb 2, 2024
14 tasks
@thw26
Copy link
Collaborator

thw26 commented Feb 8, 2024

I just got the following error while working in the code:

Installer log file: /home/thwright/.local/state/Logos_on_Linux/Logos_on_Linux.log
Traceback (most recent call last):
  File "/home/thwright/Dev/LogosLinuxInstaller/./LogosLinuxInstaller.py", line 313, in <module>
    main()
  File "/home/thwright/Dev/LogosLinuxInstaller/./LogosLinuxInstaller.py", line 244, in main
    utils.set_default_config()
  File "/home/thwright/Dev/LogosLinuxInstaller/utils.py", line 136, in set_default_config
    set_recommended_appimage_config()
  File "/home/thwright/Dev/LogosLinuxInstaller/utils.py", line 988, in set_recommended_appimage_config
    appimage_url = json_data[0].get('assets')[0].get('browser_download_url')
                   ~~~~~~~~~^^^
KeyError: 0

Here's the log:

2024-02-07 22:19:25 INFO: OS name: manjaro
2024-02-07 22:19:25 INFO: OS release: 23.1.1
2024-02-07 22:19:25 DEBUG: config.SUPERUSER_COMMAND = 'sudo'
2024-02-07 22:19:25 DEBUG: config.PACKAGE_MANAGER_COMMAND_INSTALL = 'pamac install --no-upgrade --no-confirm'
2024-02-07 22:19:25 DEBUG: config.PACKAGE_MANAGER_COMMAND_QUERY = 'pamac list -i | grep -E ^'
2024-02-07 22:19:25 DEBUG: config.PACKAGES = 'patch wget sed grep gawk cabextract samba bc libxml2 curl'
2024-02-07 22:19:25 DEBUG: config.L9PACKAGES = ''
2024-02-07 22:19:25 DEBUG: Download source: https://api.github.com/repos/FaithLife-Community/wine-appimages/releases
2024-02-07 22:19:25 DEBUG: Download destination: None
2024-02-07 22:19:25 DEBUG: Getting headers from https://api.github.com/repos/FaithLife-Community/wine-appimages/releases.
2024-02-07 22:19:25 DEBUG: Starting new HTTPS connection (1): api.github.com:443
2024-02-07 22:19:25 DEBUG: https://api.github.com:443 "HEAD /repos/FaithLife-Community/wine-appimages/releases HTTP/1.1" 403 0
2024-02-07 22:19:25 DEBUG: content_length = '279'
2024-02-07 22:19:25 DEBUG: content_md5 = None
2024-02-07 22:19:25 DEBUG: File size on server: 279
2024-02-07 22:19:25 DEBUG: chunk_size = 5; file_mode = 'wb'; headers = {'Accept-Encoding': 'identity'}
2024-02-07 22:19:25 INFO: Starting new download for https://api.github.com/repos/FaithLife-Community/wine-appimages/releases.
2024-02-07 22:19:25 DEBUG: Starting new HTTPS connection (1): api.github.com:443
2024-02-07 22:19:25 DEBUG: https://api.github.com:443 "GET /repos/FaithLife-Community/wine-appimages/releases HTTP/1.1" 403 279

Read next comment for solution.

@thw26
Copy link
Collaborator

thw26 commented Feb 8, 2024

Here's the issue:

2024-02-07 22:24:32 DEBUG: {'message': "API rate limit exceeded for ip.ad.dr.ess. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)", 'documentation_url': 'https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting'}

Documentation:

You can make unauthenticated requests if you are only fetching public data. Unauthenticated requests are associated with the originating IP address, not with the user or application that made the request.

The primary rate limit for unauthenticated requests is 60 requests per hour.

Authenticated users can make 5,000 requests per hour. I doubt we want to set up storing a GitHub account but we could. I obviously hit this limit because of my iterating my code to test.

Whatever we do, we probably need to account for this happening to our users. Here is my code modification to find this:

    logging.debug(f"{json_data}")
    if json_data is not None:
        appimage_url = json_data[0].get('assets')[0].get('browser_download_url')
    else:
        logging.critical(f"Could not get latest release.")

utils.py Outdated Show resolved Hide resolved
utils.py Show resolved Hide resolved
@n8marti n8marti requested a review from thw26 February 9, 2024 05:50
@thw26 thw26 merged commit 5f8b4bb into main Feb 9, 2024
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

2 participants