Page MenuHomePhabricator

It is difficult to keep track of used PhabricatorEdgeType IDs
Closed, ResolvedPublic

Description

Whilst undertaking D5846, it was non-trivial to find a pair of unoccupied IDs to use for EDGECONST values. Additionally, if users want to write their own PhabricatorEdgeType subclasses then they need to ensure that they start at a number high enough to not collide with the upstream.

Perhaps it would be better to use the class name as the type attribute for the edge?

Event Timeline

joshuaspence updated the task description. (Show Details)Jan 4 2015, 11:45 AM
joshuaspence added a project: Infrastructure.
joshuaspence added a subscriber: joshuaspence.
joshuaspence created this task.
joshuaspence raised the priority of this task from to Needs Triage.

Some vague thinking in this vein:

I imagine probably implementing a name registry application eventually, sort of like domain name registrars.

So if you're developing a third-party "Zebra" application, you go reserve "ZHED" and "ZBDY" and "ZFOT" as PHID types for your objects, and edge type allocation could live there too (and any other identifiers that we want to keep unique across applications). Then third-parties don't have to worry about type collisions.

I think there aren't many reasons to change how PHID types work (e.g., putting class names in the IDs would be super cumbersome) so that probably motivates this eventually if we get enough of an ecosystem.

Assuming that eventually gets built, stumbling along until then seems reasonable-ish, maybe.

We could use reasonably use class names (or other more-unique constants) for edge types instead of integers, but I'm a little worried about increasing the edge table size. I don't know if this concern is merited or not.

I also think there's a reasonable argument to be made that we should always put a layer of indirection between class names and identifiers, to prevent issues like T6849. I don't know how strong this argument is, but I'd be hesitant to undertake the migration without more thought.

Now that PhabricatorApplicationConfigurationPanel is modular, I'm inclined to do this:

  • Give Edges a real application entry, and have it show edge constants for the install in a panel.
  • Give PHIDs a real application entry, and have it show PHID constants for the install in a panel.
  • Give Remarkup a real application entry, and have it show inline rules + priorities for the install in a panel.

These would be a little bit hard to find / out of the way, but I think that's OK. They also make the application list a bit bigger, but we already have like 75 entries there or something so that cat isn't going back in the bag.

That sounds reasonable, although a CLI script could suffice I think. ./bin/phid types for example.

Yeah, real apps might be overkill for this.