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
flexget: 2.10.82 -> 2.12.8 #35557
flexget: 2.10.82 -> 2.12.8 #35557
Conversation
}: | ||
|
||
assert delugeSupport -> deluge != null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so unless I do an override the assertion will break? Wouldn’t it be better to set a sensitive default (no deluge support or deuge not null by default) to make a setup easier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it won't break. deluge should always be provided, unless someone explicitly does so but in that case he would have presumably disabled support. The assertion fails only when the package is called without deluge but delugeSupport is wanted. It's a common pattern used throughout nixpkgs to implement optional features.
and not test_non_ascii" | ||
''; | ||
# ~400 failures | ||
doCheck = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just out of interest: how far did you get? I tried to fix that once, but failed after some time...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't really tried because there were way too many. And I'm not entirely convinced the failures are all about sandboxing.
, transmission | ||
, deluge | ||
, config | ||
{ lib, config, python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config isn’t needed anymore, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, removed.
@rnhmjoj you’re right about the deluge assertion, sorry about that... IIRC hyper_ch from the #nixos IRC (don’t know his GitHub name) was interested in this, I can ping him tonight as I’m currently on my phone... |
@Ma27 Ok, thank you. |
Unfortunately it doesn't seem to work:
I started with an empty sqlite db and I just get that. |
Uhm, so maybe the version constaints were really necessary. |
maintainers = with lib.maintainers; [ domenkozar tari ]; | ||
broken = true; # as of 2018-02-09 | ||
license = licenses.mit; | ||
maintainers = with maintainers; [ domenkozar tari ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you use flexget on a regular base?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, never in fact. I have been trying to fix packages reported in #34775
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe some python dependencies should be be locked down with pypi2nix or so?
This is why I didn't switch to fetching from PyPi before- plus there doesn't seem to be any real benefit to getting it from PyPi over github, since we can still fetch by version number. I would probably avoid unpinning every dependency, since it makes it harder to tell what's probably a compatible version in Nix and what needs to be upgraded (or have an old version packaged). My attempt at 2.12.0 is at tari@a5d0126 for reference. It built but there were a lot of test failures that I didn't want to try to debug, nor did I try running the output (so it might fail in the same way as this one). Looking at upstream changes between 2.12.0 (per my attempt) and 2.12.8, the only important thing to us appears to be upgrades to rebulk (0.9) and guessit (2.1.4)- we'll probably need to upgrade both of those, since guessit tends to be very sensitive to rebulk's behavior. |
@tari Are you using flexget at least or plan so? At the moment I only see people voluntary fixing flexget. |
I currently use it, so it's useful to me if it works (but if it doesn't I'll find a replacement- I don't actually use many of its features). |
I use it as well... but is there really a replacement for flexget? |
Python hates me... I still have nightmares about when I try to package ocrmypdf |
I tried with 2.13.0 but it fails again... problematic seems to be howto get sqlalchemy 1.1.17 ? |
I assume that the easiest way would be to override our |
So, I tried now with this expression:
But still an issue with the db
|
Using now SQLAlchemy 1.1.10 and reverting back to flexet 2.12.11 I still get those crashes.... but it seems to work... it did download some torrent files. |
Ok, this expression works. Tested it now, however there's a few drawbacks:
What did I test? Just basic functionality... adding a list of tv shows and auto-fetch them from rss feeds and store them in the designated folder(s). I didn't test any direct interaction with transmission or deluge... neither tried to fetch from nzbs.
|
Upon further investigation, the problem with the 500 shows is becuase of SQLite limitations. By default it allows only 1000 max variables (and a problem with the current flexget code fills that with 500 shows). Other distros do set significantly higher default value for max vars. Hence I made a PR: #36293 |
af1a06f
to
0649f52
Compare
@sjau Thank you. I picked up your changes. |
I still don't know what the difference ist between pypi and directly getting the source though.... |
requirements.txt is generated from requirements.in which has fewer constraints and hopefully only direct dependencies. Would "mv requirements.in requirements.txt" before building be useful? Flexget developers made this change due to dependency issues of their own: Flexget/Flexget#1867 |
@rnhmjoj What is more desirable as nix expression?
|
@sjau I think pypi is preferred, however you’ll need to fetch it directly from source if you’d like to get the tests running |
@GrahamcOfBorg eval |
Failure on aarch64-linux (full log) Partial log (click to expand)
|
Failure on x86_64-darwin (full log) Partial log (click to expand)
|
@GrahamcOfBorg build flexget |
Success on x86_64-linux (full log) Attempted: flexget Partial log (click to expand)
|
Success on x86_64-darwin (full log) Attempted: flexget Partial log (click to expand)
|
Ok, sorry for the stupid error I didn't notice before. |
Success on x86_64-darwin (full log) Attempted: flexget Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: flexget Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: flexget Partial log (click to expand)
|
works good for me but I don't use a lot of plugins etc... so I'm happy with current state. However I noticed it's alread at 2.13.5. Maybe I should test that first. |
Success on x86_64-linux (full log) Attempted: flexget Partial log (click to expand)
|
v2.13.5 with hashsum |
Ok, done. @sjau Should I add you as a maintainer? |
no thx :) |
Success on x86_64-linux (full log) Attempted: flexget Partial log (click to expand)
|
Success on x86_64-darwin (full log) Attempted: flexget Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: flexget Partial log (click to expand)
|
maintainers = with lib.maintainers; [ domenkozar tari ]; | ||
broken = true; # as of 2018-02-09 | ||
license = licenses.mit; | ||
maintainers = with maintainers; [ ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@domenkozar @tari are you ok with this? I tried to align the maintainers field with reality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, fine with me.
Success on x86_64-linux (full log) Attempted: flexget Partial log (click to expand)
|
Success on x86_64-darwin (full log) Attempted: flexget Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: flexget Partial log (click to expand)
|
So, can we merge it now? |
Motivation for this change
Fix #34785
Things done
Notes
cc @domenkozar @tari