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

Fix: Update beta logic and query to handle empty report_url #141

Merged
merged 2 commits into from
Jan 23, 2025

Conversation

mreid-tt
Copy link
Contributor

@mreid-tt mreid-tt commented Jan 20, 2025

  • Updated the beta hybrid property in the Version model to treat both None and empty strings as non-beta.
  • Modified the catalog query to include versions where report_url is either NULL or an empty string when beta=False.
  • Ensured consistent behavior for beta builds in both Python and SQLAlchemy contexts.

Fixes: #140

@mreid-tt mreid-tt self-assigned this Jan 20, 2025
@mreid-tt mreid-tt changed the title Fix: Improve beta property to handle None and empty report_url Fix: Update beta logic and query to handle empty report_url Jan 20, 2025
@mreid-tt
Copy link
Contributor Author

@hgy59, I’ve tested the updates in my test environment and believe I’ve resolved the issue while also covering some edge cases. Here are the details:

Uploaded Versions

I uploaded two versions of Monit to the test environment. They are listed as follows:

Screenshot 2025-01-20 at 7 22 37 PM

Admin Interface Update

The admin interface now correctly reflects the beta status. Previously, both versions were incorrectly marked as beta. This has been resolved:

Screenshot 2025-01-20 at 7 23 44 PM

Synology Device Lookups

The lookups performed by Synology devices have also been updated.

  • Monit v5.17.1-8:

    • The normal query URL does not include Monit: http://localhost:5000/nas/?build=4458&arch=88f628x&language=enu
    • The beta query URL correctly includes Monit: http://localhost:5000/nas/?build=4458&arch=88f628x&language=enu&package_update_channel=beta
    • This matches the previous behavior for the beta channel.
  • Monit v5.29.0-10:

    • The normal query URL now correctly includes this version: http://localhost:5000/nas/?build=15047&arch=88f628x&language=enu.
      This is consistent with its non-beta status as defined by the report_url.
    • The beta query URL still includes this version: http://localhost:5000/nas/?build=15047&arch=88f628x&language=enu&package_update_channel=beta
      This behavior remains unchanged for the beta channel.

Let me know if further adjustments are needed.

@mreid-tt
Copy link
Contributor Author

mreid-tt commented Jan 22, 2025

hey @publicarray, after this is merged, I believe we would need to publish a new spkrepo version and update the server. Would you be able to assist with this?

@mreid-tt mreid-tt merged commit 32314a3 into SynoCommunity:main Jan 23, 2025
2 checks passed
@mreid-tt mreid-tt deleted the beta_flag_fix branch January 23, 2025 23:16
@publicarray
Copy link
Member

@mreid-tt i'm happy to do work on this on the weekend.

@mreid-tt
Copy link
Contributor Author

mreid-tt commented Feb 4, 2025

@mreid-tt i'm happy to do work on this on the weekend.

thanks @publicarray, let me know if I can be of assistance.

@hgy59
Copy link
Contributor

hgy59 commented Feb 20, 2025

@mreid-tt the latest update to v0.2.7 was reverted because it broke the package download.

I don't know whether it is related to this PR.
But if you have another look at beta packages, I found another issue, that might not be fixed with this:

Beta Packages are lacking the changelog and the Update Button in DSM Package center.
The changelog of the latest Monit package is

Version 5.34.4-11 beta
    1. Update monit to v5.34.4
    2. Update OpenSSL to v3.1.7.

But in Package Center it is empty and the Update Button is missing:
grafik

If the broken downloads are not related to the changes here, we need to create a new issue...

Can you double check this with the updated version?

And Monit package is only shown if already installed. It is lacking under the Community Packages in DSM. (Ok this is a hint that it might be fixed with this PR).
grafik

@mreid-tt
Copy link
Contributor Author

mreid-tt commented Feb 20, 2025

@hgy59 Thanks for the heads-up! I wasn’t aware we attempted to deploy this version or that there were issues — I never received an email or alert about #144.

Regarding your comment on breaking package downloads, I’ll need more details on what happened. Ideally, you can simulate what DSM receives from the package repository using similar link constructions. The general structure is:

  • http://localhost:5000/nas/? (local environment) or https://packages.synocommunity.com/? (live server)
  • build=15047& (DSM build version)
  • arch=88f628x& (CPU architecture)
  • language=enu& (NAS language setting)
  • package_update_channel=beta (optional beta channel)

It would help to check whether the package appears in the results and what download URL is provided. My testing for this PR was limited to verifying the versions in the index.

For example, the live server should return a link like this:

"link": "https://packages.synocommunity.com/monit/10/monit.v10.f15047%5B88f628x%5D.spk?arch=88f628x&build=15047"

The key question is whether this link was incorrect due to v0.2.7 or if the link was correct but resulted in a server error. I haven’t tested using DSM as a client locally — just checked the server output via a browser.

@publicarray, if you have any additional insights on how downloads were broken, feel free to add them to #144. That would help with diagnosing the issue.

Regarding the changelog, it should also be present in the JSON response. The current online version returns:

"changelog": "1. Update monit to v5.29.0<br/>2. Update OpenSSL to v1.1.1l"

My test setup included a value, but I can’t simulate your exact configuration without your DSM details. I suggest building a test URL, downloading it via a browser, and checking for the "package": "monit" entry in the dataset. If something seems off, opening a new issue might be the best course of action.

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.

BETA flag not shown
3 participants