nextcloud / news-updater

:newspaper: Fast, parallel feed updater for the News app; written in Python
GNU General Public License v3.0
107 stars 24 forks source link

occ news:updater:update-feed is being called with arguments reversed #26

Closed Marcool04 closed 3 years ago

Marcool04 commented 3 years ago

Hi I have noticed that news-updater has been throwing an error recently:

Oct 29 11:43:16 HOSTNAME nextcloud-news-updater[6372]: subprocess.CalledProcessError: Command '['/usr/bin/php', '-f', '/usr/share/webapps/nextcloud/occ', 'news:updater:update-feed', '7', 'USERNAME']' returned non-zero exit status 1.
Oct 29 11:43:16 HOSTNAME nextcloud-news-updater[6372]: Traceback (most recent call last):
Oct 29 11:43:16 HOSTNAME nextcloud-news-updater[6372]:   File "/usr/lib/python3.8/site-packages/nextcloud_news_updater/api/updater.py", line 34, in run
Oct 29 11:43:16 HOSTNAME nextcloud-news-updater[6372]:     self.update_feed(feed)
Oct 29 11:43:16 HOSTNAME nextcloud-news-updater[6372]:   File "/usr/lib/python3.8/site-packages/nextcloud_news_updater/api/cli.py", line 80, in update_feed
Oct 29 11:43:16 HOSTNAME nextcloud-news-updater[6372]:     self.cli.run(command)
Oct 29 11:43:16 HOSTNAME nextcloud-news-updater[6372]:   File "/usr/lib/python3.8/site-packages/nextcloud_news_updater/api/cli.py", line 12, in run
Oct 29 11:43:16 HOSTNAME nextcloud-news-updater[6372]:     return check_output(commands)
Oct 29 11:43:16 HOSTNAME nextcloud-news-updater[6372]:   File "/usr/lib/python3.8/subprocess.py", line 411, in check_output
Oct 29 11:43:16 HOSTNAME nextcloud-news-updater[6372]:     return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
Oct 29 11:43:16 HOSTNAME nextcloud-news-updater[6372]:   File "/usr/lib/python3.8/subprocess.py", line 512, in run
Oct 29 11:43:16 HOSTNAME nextcloud-news-updater[6372]:     raise CalledProcessError(retcode, process.args,

Notice that the call to occ is in the form '/usr/share/webapps/nextcloud/occ', 'news:updater:update-feed', '7', 'USERNAME' so feed-id precedes userid This seems strange since right now we have:

# runuser -l http -c "/usr/bin/php -f /usr/share/webapps/nextcloud/occ news:updater:update-feed --help"
Description:
  Console API for updating a single user's feed

Usage:
  news:updater:update-feed <user-id> <feed-id>

Arguments:
  user-id               user id of a user, string
  feed-id               feed id, integer

This is related to a 2 month old change in News: those two arguments seem to have been switched around by this commit https://github.com/nextcloud/news/commit/d00d1ab2a28f428223e52b17052c072c64784016#diff-c88d680a82ffa9ce049e6965c5102dbda37d3ca0f4d7d2ff657e3008bf703de1R39 for no apparent reason I might add…

Not sure if we should file a bug against News app or if its best to fix news-updater…

Thanks for all your work on this helpful utility. Regards, Mark.

BernhardPosselt commented 3 years ago

@DriverXX can you check?

Marcool04 commented 3 years ago

Ok, this seems to be related to News version 15. For some reason, while I am trying to update a v15 News app here, the codepath for non-15 version is being followed:

class CliUpdateThread(UpdateThread):
    def __init__(self, feeds: List[Feed], logger: Logger, api: CliApi,
                 cli: Cli) -> None:
        super().__init__(feeds, logger)
        self.cli = cli
        self.api = api

    def update_feed(self, feed: Feed) -> None:
        command = self.api.update_feed_command + [str(feed.feed_id),
                                                  feed.user_id]
        self.logger.info('Running update command: %s' % ' '.join(command))
        self.cli.run(command)

instead of this one:

class CliUpdateThreadV15(CliUpdateThread):
    """Cli Updater for Nextcloud News v15+"""

    def update_feed(self, feed: Feed) -> None:
        command = self.api.update_feed_command + [feed.user_id,
                                                  str(feed.feed_id)]
        self.logger.info('Running update command: %s' % ' '.join(command))
        self.cli.run(command)

from https://github.com/nextcloud/news-updater/blob/master/nextcloud_news_updater/api/cli.py#L75

This classname, CliUpdateThreadV15, on the other hand, is nowhere else to be found in the repo…

Marcool04 commented 3 years ago

Oh and, btw –but maybe this is a separate issue (or working as intended?): this error is visible in the logs, but strangely the process actually finishes successfully, and fails to kick the systemd .service file into a "failed" state, which would have caught my attention sooner probably

Marcool04 commented 3 years ago

Ok, very sorry, this is a configuration error. I didn't realize there was the apilevel configuration option. Of course, setting this to "v15" just works fine. I thought the API version was determined from the server News app. Should have read through the code a bit more before posting an issue. Sorry for the bother and thanks once more for all your work people!

Regards,

Mark.