SynoCommunity / spkrepo

Synology Package Repository
http://spkrepo.readthedocs.org
MIT License
155 stars 26 forks source link

Deactivated packages listed on website #97

Closed mreid-tt closed 1 year ago

mreid-tt commented 1 year ago

In the website URL https://synocommunity.com/package/[package] there is listed some packages which are listed even though they are deactivated in the admin interface. Looking at SickBeard we see the following:

Admin status:

Screenshot 2023-04-26 at 7 45 30 AM

Website status:

Screenshot 2023-04-26 at 7 45 43 AM

We should not list versions that are not active. Moreover we should not list packages if none of their versions are active.

EDIT: Looking at the code, apparently the second issue was something known as missing:

https://github.com/SynoCommunity/spkrepo/blob/76951e0e5850504b16c9164b97a23883488269d6/spkrepo/views/frontend.py#L98-L99

mreid-tt commented 1 year ago

@Diaoul, I've authored two PRs (#98, #99) which together should resolve this issue. I'd appreciate your thoughts on my approach.

mreid-tt commented 1 year ago

hey @Diaoul, I'm not very familiar with the back-end system. What are the next steps to integrating these PRs into the production system?

Diaoul commented 1 year ago

I think we need to create a new release. I have GitHub actions to push the relevant docker images. Then on the server a few docker-compose commands should do the trick.

mreid-tt commented 1 year ago

I think we need to create a new release. I have GitHub actions to push the relevant docker images. Then on the server a few docker-compose commands should do the trick.

Hey @Diaoul, I'm not very familiar with releases but I've tried to create a draft (https://github.com/SynoCommunity/spkrepo/releases/tag/untagged-d15132cea06383933b7b). Let me know what you think. It asks for binaries to attach but with the build workflow not working do I just attach the zip file for the codebase?

Diaoul commented 1 year ago

It's building

mreid-tt commented 1 year ago

It's building

Exciting! It looks like there needs to be a version number change somewhere else... the logs seem to be showing an upload attempt of the old version number:

Uploading spkrepo-0.2.2-py3-none-any.whl 100%
HTTP Error 400: File already exists.
Diaoul commented 1 year ago

Here. My bad I did not check as the tag was present.

Diaoul commented 1 year ago

Done, it's building again

Diaoul commented 1 year ago

I'll run the update on the server later tonight.

Diaoul commented 1 year ago

Upgraded the server. Can you check it is working as you expect?

mreid-tt commented 1 year ago

Upgraded the server. Can you check it is working as you expect?

I may have to give some time for the caches to update to confirm. Right now I suspect the back-end is a bit busy because I'm getting a mix of "Error 503 first byte timeout" and "Internal Server Error".

EDIT: There were also a few other commits since the last release which I don't see PRs for so I hope these aren't causing issues now that the new release is out (which should include these):

https://github.com/SynoCommunity/spkrepo/commit/987e9187d4a7ff9f944706f507b6ed4a72a3a31b https://github.com/SynoCommunity/spkrepo/commit/645918e4df80e77a5bf32a99421835c39658da70 https://github.com/SynoCommunity/spkrepo/commit/4640a220dfaef507d05f1ea89065c8c94a9c81ba https://github.com/SynoCommunity/spkrepo/commit/43e52667267b773353462af5fad613f05fd39d7a https://github.com/SynoCommunity/spkrepo/commit/76951e0e5850504b16c9164b97a23883488269d6

Diaoul commented 1 year ago


Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/gunicorn/workers/sync.py", line 136, in handle
    self.handle_request(listener, req, client, addr)
  File "/usr/local/lib/python3.9/site-packages/gunicorn/workers/sync.py", line 179, in handle_request
    respiter = self.wsgi(environ, resp.start_response)
  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 2091, in __call__
    return self.wsgi_app(environ, start_response)
  File "/usr/local/lib/python3.9/site-packages/werkzeug/middleware/proxy_fix.py", line 187, in __call__
    return self.app(environ, start_response)
  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 2076, in wsgi_app
    response = self.handle_exception(e)
  File "/usr/local/lib/python3.9/site-packages/flask_restful/__init__.py", line 271, in error_router
    return original_handler(e)
  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 2073, in wsgi_app
    response = self.full_dispatch_request()
  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 1518, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/usr/local/lib/python3.9/site-packages/flask_restful/__init__.py", line 271, in error_router
    return original_handler(e)
  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 1514, in full_dispatch_request
    rv = self.preprocess_request()
  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 1857, in preprocess_request
    rv = self.ensure_sync(before_func)()
  File "/usr/local/lib/python3.9/site-packages/flask_principal.py", line 477, in _on_before_request
    identity = loader()
  File "/usr/local/lib/python3.9/site-packages/flask_security/core.py", line 245, in _identity_loader
    if not isinstance(current_user._get_current_object(), AnonymousUserMixin):
  File "/usr/local/lib/python3.9/site-packages/werkzeug/local.py", line 565, in _get_current_object
    return self.__local()  # type: ignore
  File "/usr/local/lib/python3.9/site-packages/flask_login/utils.py", line 26, in <lambda>
    current_user = LocalProxy(lambda: _get_user())
  File "/usr/local/lib/python3.9/site-packages/flask_login/utils.py", line 346, in _get_user
    current_app.login_manager._load_user()
  File "/usr/local/lib/python3.9/site-packages/flask_login/login_manager.py", line 318, in _load_user
    user = self._user_callback(user_id)
  File "/usr/local/lib/python3.9/site-packages/flask_security/core.py", line 221, in _user_loader
    return _security.datastore.find_user(id=user_id)
  File "/usr/local/lib/python3.9/site-packages/flask_security/datastore.py", line 254, in find_user
    return self.user_model.query.filter_by(**kwargs).first()
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/orm/query.py", line 2810, in first
    return self.limit(1)._iter().first()
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/orm/query.py", line 2894, in _iter
    result = self.session.execute(
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/orm/session.py", line 1692, in execute
    result = conn._execute_20(statement, params or {}, execution_options)
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1620, in _execute_20
    return meth(self, args_10style, kwargs_10style, execution_options)
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/sql/elements.py", line 325, in _execute_on_connection
    return connection._execute_clauseelement(
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1487, in _execute_clauseelement
    ret = self._execute_context(
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1851, in _execute_context
    self._handle_dbapi_exception(
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 2032, in _handle_dbapi_exception
    util.raise_(
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/util/compat.py", line 207, in raise_
    raise exception
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1808, in _execute_context
    self.dialect.do_execute(
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/default.py", line 732, in do_execute
    cursor.execute(statement, parameters)
sqlalchemy.exc.ProgrammingError: (psycopg2.errors.UndefinedColumn) column user.fs_uniquifier does not exist
LINE 2: ....github_access_token AS user_github_access_token, "user".fs_...```
mreid-tt commented 1 year ago

This looks like the commit for https://github.com/SynoCommunity/spkrepo/commit/76951e0e5850504b16c9164b97a23883488269d6. Did you roll it back?

EDIT: Or perhaps it could be this:

https://github.com/SynoCommunity/spkrepo/blob/43e52667267b773353462af5fad613f05fd39d7a/spkrepo/models.py#L38

Which was implemented as part of https://github.com/SynoCommunity/spkrepo/commit/43e52667267b773353462af5fad613f05fd39d7a.

mreid-tt commented 1 year ago

EDIT: There were also a few other commits since the last release which I don't see PRs for so I hope these aren't causing issues now that the new release is out (which should include these):

987e918 645918e 4640a22 43e5266 76951e0

@publicarray, can you comment on these commits? Based on the error above, one of these may be causing an error on the server.

Diaoul commented 1 year ago

I've pushed a wip branch that I had locally that fixes an issue with the latest flask-security-too, maybe it's worth getting it some love. Currently some tests are failing but I think that's fixable.

publicarray commented 1 year ago


Traceback (most recent call last):

  File "/usr/local/lib/python3.9/site-packages/gunicorn/workers/sync.py", line 136, in handle

    self.handle_request(listener, req, client, addr)

  File "/usr/local/lib/python3.9/site-packages/gunicorn/workers/sync.py", line 179, in handle_request

    respiter = self.wsgi(environ, resp.start_response)

  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 2091, in __call__

    return self.wsgi_app(environ, start_response)

  File "/usr/local/lib/python3.9/site-packages/werkzeug/middleware/proxy_fix.py", line 187, in __call__

    return self.app(environ, start_response)

  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 2076, in wsgi_app

    response = self.handle_exception(e)

  File "/usr/local/lib/python3.9/site-packages/flask_restful/__init__.py", line 271, in error_router

    return original_handler(e)

  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 2073, in wsgi_app

    response = self.full_dispatch_request()

  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 1518, in full_dispatch_request

    rv = self.handle_user_exception(e)

  File "/usr/local/lib/python3.9/site-packages/flask_restful/__init__.py", line 271, in error_router

    return original_handler(e)

  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 1514, in full_dispatch_request

    rv = self.preprocess_request()

  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 1857, in preprocess_request

    rv = self.ensure_sync(before_func)()

  File "/usr/local/lib/python3.9/site-packages/flask_principal.py", line 477, in _on_before_request

    identity = loader()

  File "/usr/local/lib/python3.9/site-packages/flask_security/core.py", line 245, in _identity_loader

    if not isinstance(current_user._get_current_object(), AnonymousUserMixin):

  File "/usr/local/lib/python3.9/site-packages/werkzeug/local.py", line 565, in _get_current_object

    return self.__local()  # type: ignore

  File "/usr/local/lib/python3.9/site-packages/flask_login/utils.py", line 26, in <lambda>

    current_user = LocalProxy(lambda: _get_user())

  File "/usr/local/lib/python3.9/site-packages/flask_login/utils.py", line 346, in _get_user

    current_app.login_manager._load_user()

  File "/usr/local/lib/python3.9/site-packages/flask_login/login_manager.py", line 318, in _load_user

    user = self._user_callback(user_id)

  File "/usr/local/lib/python3.9/site-packages/flask_security/core.py", line 221, in _user_loader

    return _security.datastore.find_user(id=user_id)

  File "/usr/local/lib/python3.9/site-packages/flask_security/datastore.py", line 254, in find_user

    return self.user_model.query.filter_by(**kwargs).first()

  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/orm/query.py", line 2810, in first

    return self.limit(1)._iter().first()

  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/orm/query.py", line 2894, in _iter

    result = self.session.execute(

  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/orm/session.py", line 1692, in execute

    result = conn._execute_20(statement, params or {}, execution_options)

  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1620, in _execute_20

    return meth(self, args_10style, kwargs_10style, execution_options)

  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/sql/elements.py", line 325, in _execute_on_connection

    return connection._execute_clauseelement(

  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1487, in _execute_clauseelement

    ret = self._execute_context(

  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1851, in _execute_context

    self._handle_dbapi_exception(

  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 2032, in _handle_dbapi_exception

    util.raise_(

  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/util/compat.py", line 207, in raise_

    raise exception

  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1808, in _execute_context

    self.dialect.do_execute(

  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/default.py", line 732, in do_execute

    cursor.execute(statement, parameters)

sqlalchemy.exc.ProgrammingError: (psycopg2.errors.UndefinedColumn) column user.fs_uniquifier does not exist

LINE 2: ....github_access_token AS user_github_access_token, "user".fs_...```

Looks like the fs_uniquifirr column needs to be added to the database

Diaoul commented 1 year ago

Weird, I think I reverted the deployment and stuck to the old release instead 🤔

th0ma7 commented 1 year ago

That would explain why my python updates still showed up while not yet activated...

mreid-tt commented 1 year ago

@mreid-tt IMHO it was by intention that a manual download of deactivated packages was possible. The former publish workflow was to manually download, install and test the packages before activation.

@hgy59, thanks for the perspective. If the download, install and test workflow was for admins then there should be a way for admins to do so via the admin portal (maybe on the build page they could be hyperlinked). If on the other hand, this was supposed to be viewable by the public then I don't see the point of having an activate function at all.

hgy59 commented 1 year ago

If on the other hand, this was supposed to be viewable by the public then I don't see the point of having an activate function at all.

Inactive packages are not visible in the package center of DSM.

mreid-tt commented 1 year ago

Inactive packages are not visible in the package center of DSM.

Really? So when we had this issue (https://github.com/SynoCommunity/spksrc/issues/5798#issuecomment-1626253928) we could have just disabled it rather than deleting it?

hgy59 commented 1 year ago

There are still use cases that require the old behaviour. https://github.com/SynoCommunity/spksrc/issues/5684#issuecomment-1714305333