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

Running Pepys Admin before Import creates database at wrong version #1094

Open
IanMayo opened this issue Nov 29, 2021 · 2 comments
Open

Running Pepys Admin before Import creates database at wrong version #1094

IanMayo opened this issue Nov 29, 2021 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@IanMayo
Copy link
Member

IanMayo commented Nov 29, 2021

Version

0.0.33

🐞 Description

I've just encountered something strange. It's repeatable, though I don't fully understand the reason.

⚙️ Current behavior

Running Pepys Admin before Pepys Import results in database at unexpected version

⚙️ Expected behavior

It should be possible to run pepys admin then pepys import without errors.

Note: I can delete the .sqlite file generated by Pepys-Admin. Then when I run pepys-import it happily (successfully) generates the database file and runs the import.

🔢 Steps to Reproduce

  • freshly unzipped Pepys Deployment release (with default config file)
  • run install_pepys.bat, see confirmation that shortcuts created
  • run Pepys Admin from start meu
  • run 2. Status
  • see confirmation that Pepys creating database tables by Alembic
  • exit Pepys Admin
  • verify sqlite database created in bin folder
  • navigate to sample data
  • open NMEA folder
  • right-click on NMEA_trial.log, select Send to > Pepys (No archive)

🖥️ Screenshots

image

@IanMayo IanMayo added the bug Something isn't working label Nov 29, 2021
@robintw
Copy link
Collaborator

robintw commented Nov 29, 2021

The reason for this problem is that running the Status command does not create/migrate the tables in the database. The Database tables are going to be created by Alembic. message is misleading - it is actually saying that we are running alembic (but not to do a migration, just to tell us what the current state of the database is according to alembic) and it is creating the relevant spatial tables (for Spatialite/PostGIS) rather than the Pepys tables.

I think the solution is to make it clear that the Status command does not create the tables for you - for that you need to run Initialise or Migrate. I think we should just remove the Database tables are going to be created by Alembic. as it isn't helpful - and in other situations where it does apply, there will be other output that shows that the tables are being created.

The current output of running Status for a database that doesn't exist yet is:

--- Menu ---
(1) Initialise/Clear
(2) Status
(3) Export
(4) Snapshot
(5) Migrate
(6) View Data
(7) View Docs
(8) Maintenance
(9) Maintain tasks
(10) View dashboard
(.) Exit

(pepys-admin) 2
Database tables are not found! (Hint: Did you initialise the DataStore?)
## Database Version
Database tables are not found! (Hint: Did you initialise the DataStore?)
Database tables are going to be created by Alembic.
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
Current revision(s) for sqlite+pysqlite://:@:0/bug3.db:
## Config file
Location: /Users/robin/Documents/IanMayo/pepys_config.ini
Contents:
...

I think the only misleading thing in there is it saying that the tables are going to be created (which they won't be). Ideally we would change the Current revision(s) line to explicitly say There is no revision in the database at the moment - but that line comes directly from an alembic command and we can't change it.

What do you think?

@IanMayo
Copy link
Member Author

IanMayo commented Nov 29, 2021

Aah, in that case I think the issue is that Pepys-Admin creates the database, but doesn't populate it.

Then when Pepys-Import runs, it thinks the database is in an unexpected state - and it drops out.

Ok, maybe this isn't a software issue, but a training issue. Yes - some confusion could be avoided by removing the Database tables are going to be created by Alembic statement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants