Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit hacky script to visualise store inheritance #11357

Closed
wants to merge 4 commits into from

Conversation

DMRobertson
Copy link
Contributor

Use e.g. with scripts-dev/storage_inheritance.py DataStore --show.

It's dirty, but it's useful enough to help disentangle the sphagetti.

Use e.g. with `scripts-dev/storage_inheritance.py DataStore --show`.
@DMRobertson DMRobertson requested a review from a team as a code owner November 16, 2021 13:52
@DMRobertson
Copy link
Contributor Author

scripts-dev/storage_inheritance.py DataStore --show running at 9e361c8:

image

scripts-dev/storage_inheritance.py BaseSlavedStore --show at cd7257bf27b0344266ae507085066c9c6c604b6d:

image

scripts-dev/storage_inheritance.py --show at 9e361c8:

image

import tempfile
from typing import Iterable, Optional, Set

import networkx
Copy link
Contributor

@babolivier babolivier Nov 16, 2021

Choose a reason for hiding this comment

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

We should add networkx to the dev dependencies.

@babolivier babolivier requested a review from a team November 16, 2021 13:59
"synapse",
"tests",
],
cwd="/home/dmr/workspace/synapse/",
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume we probably want to change this so it doesn't point to your home directory.

@babolivier babolivier requested a review from a team November 16, 2021 14:00
@babolivier
Copy link
Contributor

Also I would suggest writing the file on disk rather than opening it. For example I know Gnome has some weird magic around default apps, and when I tried running the script it opened it with Gimp for some reason despite the fact that I've changed the default app for PNGs to something else some time ago.

@DMRobertson
Copy link
Contributor Author

Also I would suggest writing the file on disk rather than opening it.

There's a command line option for this: --output DEST.

For example I know Gnome has some weird magic around default apps, and when I tried running the script it opened it with Gimp for some reason despite the fact that I've changed the default app for PNGs to something else some time ago.

Suggest checking your config with:

$ xdg-mime query default image/png
org.gnome.eog.desktop

@babolivier
Copy link
Contributor

There's a command line option for this: --output DEST.

Hah, I see. Maybe worth adding some doc along with the script so we remember how to use it, then? Otherwise I'm afraid it'll become like the config validator/parser util in Synapse and we'll completely forget it's a thing until someone complains we don't have such a tool or it's broken.

@DMRobertson
Copy link
Contributor Author

Hah, I see. Maybe worth adding some doc along with the script so we remember how to use it, then?

I use argparse so there's a --help out of the box for this

@anoadragon453
Copy link
Member

What's the expected lifetime of this script? Do we expect to remove it once DataStore is less 🍝?

@clokep
Copy link
Member

clokep commented Nov 18, 2021

I'm going to remove this from the review queue since it seems to be awaiting info from @DMRobertson.

@clokep clokep removed the request for review from a team November 18, 2021 18:12
@DMRobertson
Copy link
Contributor Author

What's the expected lifetime of this script? Do we expect to remove it once DataStore is less spaghetti?

I hadn't really considered it, but that sounds sensible.

(I can imagine a version that imports a class and inspects __mro__ attributes to build this graph, but hopefully we aren't relying on inheritance to this extent anywhere else?)

@DMRobertson
Copy link
Contributor Author

I'm going to close this for now since I don't think we're getting to this any time soon.

If we want to tackle our monstrous store classes we can resurrect this if it's useful.

Cross-refs: #11733, #11165

@DMRobertson DMRobertson closed this May 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants