Skip to content

chore: allow development without DB reconnection#25

Merged
albireox merged 5 commits intomainfrom
albireox/dev-no-reconnect
Jun 26, 2024
Merged

chore: allow development without DB reconnection#25
albireox merged 5 commits intomainfrom
albireox/dev-no-reconnect

Conversation

@albireox
Copy link
Member

@albireox albireox commented Jun 25, 2024

Adds some additional options to facilitate development without the DB connection having to be recreated for every query (which takes a long time with Peewee using a tunnel as the reconnection forces the models to be re-reflected).

  • sdssdb does not try to connect to the default profile when the database is imported. In development this will fail but take a long time because pipelines.sdss.org is not reacheable. This should be ok since every query has get_pw_db as a dependency.

  • If db_server is None, it does not try to connect to a profile and uses db_user, db_host, etc.

  • If db_reset is False, it does not close the connection, and the next time the DB is required it does not try to reconnect.

For development one would edit ~/.config/sdss/valis.yaml and add

db_server: null
db_reset: false
db_user: ...
db_port: ...
...

Without a valis.yaml file or environment variables everything should behave as it did.

@albireox albireox requested a review from havok2063 as a code owner June 25, 2024 21:16
Copy link
Contributor

@havok2063 havok2063 left a comment

Choose a reason for hiding this comment

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

So to clarify, does this change remove the need to do that local hack when developing, the one where we setup the db globally and remove the specific dependencies on each route?

For local development using the tunnel, I would set db_reset: false, but for production we'd set db_reset: true? Is that correct?

@albireox
Copy link
Member Author

Yes, that's all correct. For production db_reset: true (which is the default) should not change anything about how things work. The DB is still reset after each use, and it will try to connect to pipelines by default. But now if you have a custom user/host/... it will use those to begin with instead of trying pipelines first and only the custom config after that fails. And with db_reset: false it won't close the DB connection so successive queries will not have to reconnect and re-reflect the models.

I think I have also found a better way of handling the reflection in sdssdb for Peewee that speeds up things a lot because instead of having to do one query for each table to reflect the model, now it gets all the information it needs for the entire schema and caches it. I'm still testing it and making sure it doesn't break things, but if it doesn't I'll tag a new sdssdb.

@havok2063 havok2063 self-requested a review June 26, 2024 16:50
Copy link
Contributor

@havok2063 havok2063 left a comment

Choose a reason for hiding this comment

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

That all sounds good to me. Thanks for suggesting a fix for this. It was a tad annoying having to tweaks things locally and not accidentally commit them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants