-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
ref!: various upgrades and fixes #102
Conversation
I'm not that versed in Python so I can't comment on a lot of this. I do however see code for the missing 'fs_uniquifier' columns as part of the upgrade() function. Hopefully this resolves the application errors you cited. |
LGTM |
@Diaoul I am trying to follow the Installation instructions for Development I get an error when installing the dependencies with poetry: Installing dependencies from lock file
Unable to read the lock file (Invalid statement (at line 428, column 1)). indeed there are some git diff markers (like EDIT: |
Fixed a bunch of things, now crashing later down the process during tests. Feel free to give it another shot 👍 |
Hey @Diaoul, eager to see further developments on this. |
To fix the display of the Startable attribute, the definition of the startable field in the version table should be changed (add default=True) in models.py
|
hey @Diaoul, I've been trying out the instructions in the new readme and I initially got stuck on the database table creation. I found a bug in the sequence for the migrations and corrected with b38660d. I'm now stuck at adding the admin account but I'm not sure how to fix. I see this:
As you can see it is complaining that the 'username' field is not determined even though it's in the command. I've been able to manually create a user with SQL and continue all the way to the login but I suspect this is an issue which may be contributing to other failures. Any thoughts on resolving? P.S. I've also tried a number of variations of the command including the password option before the attributes but this had no effect. |
I am facing the same issue. I solved it by creating the user with the following query:
I had to set the With this, I was able to complete the installation with
BTW: when I tried to create the user with With this user, I am able to login into the admin pages. |
The next error that pops up: From the logs of spkrepo I assume it is caused by a missing fs_uniquifier property. |
I also worked around the issue by simply registering the user at http://localhost:5000/register. Once the command was fired a user was created (along with the
I am also able to login to the admin pages but my raising this issue was more to address the root cause rather than document a workaround. I suspect something about the definition of how the app creates a user is not well defined and this is causing a problem with the flask command. |
Fixed by including the additional required parameters for |
- Use explicit login with non-hashed password
@publicarray all tests successful now. I think we need to update the version of the dependency pillow from 8.3.2 to 9.5.0 which is compatible with Python 3.11. I'm not familiar with updating these dependencies so I just manually did this in my test environment:
EDIT: I think I've updated it. However I think I broke something. On further checking it seems that the dependency for Werkzeug needs to be restricted to less than 3.0.0 as this version causes most of the api test post checks to fail. EDIT: For future reference the process to update the dependencies is as follows:
Note: Once you merge there may be a pre-commit check error which may need a manual adjustment in the |
- Werkzeug >= 3.0.0 causes these tests to fail - Correct fake api_key length
@publicarray, I've rebuilt my environment and ran a number of tests which all look good. I didn't use docker as stated in the readme but the rest of it was pretty much the same. Below are my logs for reference: InstallationPython Install
Source repo install
App environment setup
App initial setup Create database
Populate database
Setup admin account Create account
Add account roles
Run and TestLaunch app
Test package upload
Execute test suite
All in all things look good and we should be good to merge. |
from 140 passed, 41 warnings in 56.83s to 140 passed, 6 warnings in 53.66s
- Removes restriction on Werkzeug version by upgrading HTTP headers
docker-compose is nowadays included with docker depopulate_db deletes packages from the database ensuring the filesystem is in-sync with the database
This reverts commit 4247322.
Thanks guys! Awesome job! 👏 |
Fixes #106