Skip to content

Postgres K8s integration#9284

Merged
abyesilyurt merged 49 commits intoaziz/sql_jsonfrom
shubham/postgres-k8s
Sep 12, 2024
Merged

Postgres K8s integration#9284
abyesilyurt merged 49 commits intoaziz/sql_jsonfrom
shubham/postgres-k8s

Conversation

@shubham3121
Copy link
Member

Description

Please include a summary of the change, the motivation, and any additional context that will help others understand your PR. If it closes one or more open issues, please tag them as described here.

Affected Dependencies

List any dependencies that are required for this change.

How has this been tested?

  • Describe the tests that you ran to verify your changes.
  • Provide instructions so we can reproduce.
  • List any relevant details for your test configuration.

Checklist

kiendang and others added 30 commits August 26, 2024 12:31
- add environment and settings variables for postgres
- new error handling for postgresql store
- add postgres service to devspace
- pass postgres creds to backend

Co-authored-by: khoaguin <dkn.work@protonmail.com>
…ingStore`

[tox] trying to delete a datasite cluster before launching
[chores] improve some debug printings
…d postgre backing store

- postgre's cursor is not cached and renewed whenever a command is executed

Co-authored-by: Shubham Gupta <shubhamgupta3121@gmail.com>
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

shubham3121 and others added 7 commits September 12, 2024 11:23
@shubham3121 shubham3121 added the Type: Improvement 📈 Minor improvements not introducing a new feature or requiring a major refactor label Sep 12, 2024
db_config.reset = True

self.db_config = db_config
self.db: PostgresDBManager | SQLiteDBManager | None = None
Copy link
Contributor

Choose a reason for hiding this comment

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

should be self.db: DBManager (and not nullable)

perhaps:

self.db = self.init_db(db_config)

Copy link
Member Author

Choose a reason for hiding this comment

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

Resolved in 000092d

server_uid=self.id,
root_verify_key=self.verify_key,
)
if isinstance(db_config, SQLiteDBConfig):
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick, perhaps we can add factory method on DBManager that handles this

Copy link
Member Author

Choose a reason for hiding this comment

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

I differ a bit on this, since there are only two types of config currently, being explicit make sense to me.

@eelcovdw
Copy link
Contributor

great! 💯

update init_stores to return db manager
@abyesilyurt abyesilyurt merged commit 71c0c02 into aziz/sql_json Sep 12, 2024
@abyesilyurt abyesilyurt deleted the shubham/postgres-k8s branch September 12, 2024 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Improvement 📈 Minor improvements not introducing a new feature or requiring a major refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants