Skip to content

[add] series CLI - use environment variables for defaults#1850

Merged
liiight merged 6 commits intodevelopfrom
series-cli-env-defaults
Jun 1, 2017
Merged

[add] series CLI - use environment variables for defaults#1850
liiight merged 6 commits intodevelopfrom
series-cli-env-defaults

Conversation

@tubedogg
Copy link
Copy Markdown
Contributor

@tubedogg tubedogg commented May 29, 2017

Motivation for changes:

Feature request

Detailed changes:

series show and series list CLI subcommands now read environment variables to determine default parameters. These can be overridden on a per-run basis by utilizing the existing command-line arguments.

  • flexget series show:

    • FLEXGET_SERIES_SHOW_SORTBY_FIELD can be set to age (default) or identifier to determine which field releases are sorted by. (--sort-by optional argument)
    • FLEXGET_SERIES_SHOW_SORTBY_ORDER can be set to desc to display results in descending order. (--descending/--ascending optional arguments)
  • flexget series list:

    • FLEXGET_SERIES_LIST_CONFIGURED can be set to configured (default), unconfigured, or all to determine whether to only show series which are currently in the config. (positional argument)
    • FLEXGET_SERIES_LIST_PREMIERES can be set to yes to only show series which only have episode 1, or 1 and 2, downloaded. (--premieres optional argument)
    • FLEXGET_SERIES_LIST_STATUS can be set to new to only show series that have seen a release in the last 7 days, or stale to only show series that haven't seen a release in the last 365 days. (--new/--stale optional arguments)
    • FLEXGET_SERIES_LIST_NUMDAYS can be set to the number of days that STATUS limits to. (the DAYS option in --new DAYS/--stale DAYS)
    • FLEXGET_SERIES_LIST_SORTBY_FIELD can be set to name (default) or age to determine which field series are sorted by. (--sort-by optional argument)
    • FLEXGET_SERIES_LIST_SORTBY_ORDER can be set to desc to display results in descending order. (--descending/--ascending optional arguments)

Note there are presently bugs with the output of series list --premieres and series list --sort-by age, which already existed and are unrelated to these changes. Specifically, series list --premieres doesn't take season packs into account when determining which series to display, and series list --sort-by age seems to sort-of-but-not-completely sort by age. I'll be looking at these separately and submit another PR if I can resolve them.

tubedogg added 2 commits May 29, 2017 16:59
`series show` and `series list` CLI subcommands now read environment variables to determine default parameters. These can be overridden on a per-run basis by utilizing the existing command-line arguments.
@liiight
Copy link
Copy Markdown
Member

liiight commented May 31, 2017

Maybe move var names to consts? So you'd see them all at once?

Rename `FLEXGET_SERIES_SORTBY_*` to `FLEXGET_SERIES_SHOW_SORTBY_*` to differentiate from `FLEXGET_SERIES_LIST_SORTBY_*`.
@tubedogg
Copy link
Copy Markdown
Contributor Author

Addressed in 97acac7.

default='configured',
help='Limit list to series that are currently in the config or not (default: %(default)s)')
list_parser.add_argument('--premieres', action='store_true',
list_parser.add_argument('--premieres', action='store_const', const='premieres',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why change this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a good question. I think it was about eliminating the default False that store_true creates when the flag isn't used? Because for the --descending/--ascending flags I need to know if they were present to override envs (since both --ascending being present or --descending not being present result in a value of False, so no way to tell which is the case just based on the value). In this case, since there's only one, False would mean it's not present, so that would be fine. I can change it back.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it's fine by me, i was just curious

show_order = show_parser.add_mutually_exclusive_group(required=False)
show_order.add_argument('--descending', dest='order', action='store_true', help='Sort in descending order')
show_order.add_argument('--ascending', dest='order', action='store_false', help='Sort in ascending order')
show_order.add_argument('--descending', dest='order', action='store_const', const='desc',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you could probably reuse these args if you really really wanted to :-)
you can set a parent parser and inherit it, but it's really a minor issue.

@liiight liiight merged commit 897900d into develop Jun 1, 2017
@tubedogg tubedogg deleted the series-cli-env-defaults branch June 2, 2017 03:11
@y
Copy link
Copy Markdown

y commented Jul 5, 2017

Would it be possible to set these environment variables directly from the config? That would make it easier for users since they don't have to split logic between config and shell files and easier for devs to track down problems. You could add a new item to the issues template like "show the output of env | grep FLEXGET, but that wouldn't show anything for users who set the variables directly in their crontab.

@liiight
Copy link
Copy Markdown
Member

liiight commented Jul 6, 2017

currently, no cli plugin even have access to the config. i'm not saying i'm necessarily against it, but making this change work generically and correctly is a large task IMO, and i'm not sure the added value is worth it. maybe someone else feels different though

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.

3 participants