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

Directly read from the Tiamat database #226

Merged
merged 1 commit into from
Mar 20, 2024
Merged

Conversation

culka
Copy link
Contributor

@culka culka commented Mar 18, 2024

  • Use the default database also as the metadata database
  • Create a direct connection to the Tiamat database
  • Track all public tables
  • Track all relevant parent -> child connections

Resolves HSLdevcom/jore4#1673


This change is Reviewable

@culka culka force-pushed the tiamat-read-only-database branch from 35189f0 to 145d49f Compare March 20, 2024 09:54
@culka culka marked this pull request as ready for review March 20, 2024 09:55
Copy link
Contributor

@Leitsi Leitsi left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 102 files at r1.
Reviewable status: 3 of 102 files reviewed, 2 unresolved discussions (waiting on @culka)


metadata/hsl/databases/databases.yaml line 7 at r1 (raw file):

      database_url:
        from_env: HASURA_TIAMAT_DATABASE_URL
      isolation_level: read-committed

Other connections have isolation\_level: serializable, any special reason for not using that?


metadata/hsl/databases/databases.yaml line 12 at r1 (raw file):

    naming_convention: hasura-default
    root_fields:
      namespace: stop_database

In other databases we have used plural (which would sound better anyway, in my opinion, because it sounds a bit less like a verb), so should we use stops_database here? Also, the folder name tiamatdb does not match this, is that fine?

Also database -> db would be an acceptable abbreviation I think, but this is fine as well.

Copy link
Contributor

@Leitsi Leitsi left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 of 102 files reviewed, 3 unresolved discussions (waiting on @culka)


migrations/generic/metadata/readme.txt line 1 at r1 (raw file):

Otherwise empty folder expected.

Could elaborate a little, if the purpose of these files is to ensure that the folder exists (for whatever reason).

Copy link
Contributor Author

@culka culka left a comment

Choose a reason for hiding this comment

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

Reviewed 7 of 102 files at r1.
Reviewable status: 7 of 102 files reviewed, 3 unresolved discussions (waiting on @Leitsi)


metadata/hsl/databases/databases.yaml line 7 at r1 (raw file):

Previously, Leitsi (Tommi Leinamo) wrote…

Other connections have isolation\_level: serializable, any special reason for not using that?

No real reason, this should not have any effects because writes are impossible, but let's use the same one.


metadata/hsl/databases/databases.yaml line 12 at r1 (raw file):

Previously, Leitsi (Tommi Leinamo) wrote…

In other databases we have used plural (which would sound better anyway, in my opinion, because it sounds a bit less like a verb), so should we use stops_database here? Also, the folder name tiamatdb does not match this, is that fine?

Also database -> db would be an acceptable abbreviation I think, but this is fine as well.

Changed database to stops_database
Also yeah, tiamatdb should rather be stops, changed 👍


migrations/generic/metadata/readme.txt line 1 at r1 (raw file):

Previously, Leitsi (Tommi Leinamo) wrote…

Could elaborate a little, if the purpose of these files is to ensure that the folder exists (for whatever reason).

Added a better description.

@culka culka force-pushed the tiamat-read-only-database branch from 145d49f to b86cdf5 Compare March 20, 2024 10:57
- Use the default database also as the metadata database
- Create a direct connection to the Tiamat database
- Track all public tables
- Track all relevant parent -> child connections
@culka culka force-pushed the tiamat-read-only-database branch from b86cdf5 to 9a0e835 Compare March 20, 2024 11:56
Copy link
Contributor

@Leitsi Leitsi left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 3 of 104 files reviewed, all discussions resolved (waiting on @culka)

@culka culka merged commit 73ec408 into main Mar 20, 2024
10 of 11 checks passed
@culka culka deleted the tiamat-read-only-database branch March 20, 2024 12:56
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.

Use Tiamat directly from Hasura as a read-only database
2 participants