Skip to content
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

Address missing UUIDs in pre-existing users #1042

Merged
merged 3 commits into from
Aug 25, 2023

Conversation

michplunkett
Copy link
Collaborator

@michplunkett michplunkett commented Aug 24, 2023

Description of Changes

The previous build ran into a problem where we added a middleware creation for the _uuid column in the users table, but did not deal with the problem of the rows that lacked UUIDs in the users table. This was dealt with through the use of a loop in the migration that adds a UUID to all pre-existing rows via Python 🐍.

Tests and linting

  • This branch is up-to-date with the develop branch.
  • pytest passes on my local development environment.
  • pre-commit passes on my local development environment.
  • Manually tested through the below process:
% git checkout address_missing_uuids
% make clean_all                          
rm -rf ./OpenOversight/app/static/dist/
docker-compose stop
...
% git checkout 872a01ab  # Checks out a previous version of the branch before we merged in the UUIDs
...
HEAD is now at 872a01ab Remove `HiddenField`s and add `created_by` to `LicensePlate` and `Location` (#1034)
% make start    
docker-compose build
...
% make dev # Create tables and the application at the point in time for the above commit
... # Verify that all tables and data are in place, etc.
% git checkout address_missing_uuids      
Previous HEAD position was 872a01ab Remove `HiddenField`s and add `created_by` to `LicensePlate` and `Location` (#1034)
Switched to branch 'address_missing_uuids'
Your branch is up to date with 'origin/address_missing_uuids'.
% (env) michaelp@MacBook-Air-5 OpenOversight % make start
docker-compose build
...
docker-compose up -d
WARN[0000] The "APPROVE_REGISTRATIONS" variable is not set. Defaulting to a blank string. 
[+] Running 2/0
 ✔ Container openoversight-postgres-1  Running                                                                                                                                                             0.0s 
 ✔ Container openoversight-web-1       Running                                                                                                                                                             0.0s 
% docker exec -it openoversight-web-1 bash
I have no name!@0e2d34544358:/usr/src/app$ flask db upgrade
...
INFO  [alembic.runtime.migration] Running upgrade b38c133bed3c -> a35aa1a114fa, Add create and last_update columns
INFO  [alembic.runtime.migration] Running upgrade a35aa1a114fa -> 52d3f6a21dd9, add _uuid column to users
I have no name!@0e2d34544358:/usr/src/app$ exit
%

@sea-kelp
Copy link
Collaborator

Would doing this as a separate migration cause issues for us when we try to apply changes to our instance?

@michplunkett
Copy link
Collaborator Author

The thing you'd have to make sure of is that the _uuid field isn't ever null when it's marked as non-null. Other than that, you could do this any which way you wanted, imo. That's the one limitation.

Copy link
Member

@b-meson b-meson left a comment

Choose a reason for hiding this comment

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

👍

@michplunkett michplunkett merged commit 9264bd7 into develop Aug 25, 2023
2 checks passed
@michplunkett michplunkett deleted the address_missing_uuids branch August 25, 2023 18:09
sea-kelp pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Sep 25, 2023
sea-kelp pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Sep 25, 2023
sea-kelp pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Oct 9, 2023
AetherUnbound pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Nov 11, 2023
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.

None yet

3 participants