Refactor platform table to enable custom domains #293

Activity

1. MoralCode

   added a parent issue on May 5, 2026

   • Epic: Flexible platform support #176
2. MoralCode

   added this to CollectOSS + 8Knot WG Feature Roadmapon May 5, 2026
3. github-project-automation

   moved this to Todo in CollectOSS + 8Knot WG Feature Roadmapon May 5, 2026
4. MoralCode commented on May 5, 2026

   

   MoralCode

   on May 5, 2026

   Last edited by MoralCode

   ContributorAuthor

   More actions

   Additionally, (cross references notwithstanding) it seems like this table is likely only useful during collection, and should be in the operations schema

   That, oddly, makes migration easy, as a new table can be spun up in that location independently. once the old referring tables are divorced from the foreign keys and are instead using those referring columns to denote the platform type (via an enum in the codebase *), that would allow the old platform table to be removed entirely.

   Now all we need is the schema for it

   * not in the database because they are hard to migrate in postgres.
5. MoralCode commented on May 5, 2026

   

   MoralCode

   on May 5, 2026

   Last edited by MoralCode

   ContributorAuthor

   More actions

   To get this rolling, id like to create a new table in the operations schema

   id like to call it instances (or similar) to more clearly define its purpose.

   The new table needs (first draft):

   • platform_type (the same integer being referred to by the tables mentioned above) - this would denote which API implementation gets used for the platform
   • platform_name - user-defined name for the platform, i.e. "my cool forgejo instance"/"homelab"
   • date_added - might be useful for sorting i guess
   • domain name - the domain name to use for the platform. This will be used to support Add option to use Github Enterprise API for api key checks #110 and/or custom gitlab/forgejo instances
   • enabled - denotes whether to collect for that platform (up for debate as to whether we can add this later, this idea was from the linked issue discussed below)
6. MoralCode commented on May 5, 2026

   

   MoralCode

   on May 5, 2026

   Last edited by MoralCode

   ContributorAuthor

   More actions

   Comparison with related issue

   while #71 is a decent start for this, that issue was written with a different vision for the platform table in mind (i.e. continuing using it like an enum) which differs from the current one.

   I see this issue as being the modern replacement of/superseding that issue.

   This is because I believe the _platform_s (AKA APIs) that are supported should be defined in the code. It makes no sense to allow a new platform to be added with a database table because it would ALSO require code changes to support that platform's APIs anyway - so why not have both in the same place.

   this is why, what #71 calls "platform", i call "platform type", an identifier managed in code, alongside/integrated with the platform implementations.

   this is also why id prefer the term "instances" for the table above (at least internally), since platform seems like it can be interpereted both ways

   key limiting:

   #71 suggests we add a "platform features" column to the worker_oauth table where we store API keys. This would be for storing key pseudo-platforms (IE: the different request limits for each key type - gor github, these would be rest and graphql) these values would be used by keyman for rate limiting.

   For the purposes of this issue, I would call this "rate limit groups" instead, and i think they belong in the internal implementation, alongside other information like the platform name/type, rather than in a DB table, since they are basically fixed information that changes with the API (and should therefore live in the implementation code).

   key storage:

   I generally agree with #71 on this last point. worker_oauth needs to be the only place we store API keys.
7. MoralCode

   self-assigned this

   on May 5, 2026
8. MoralCode

   added

   discussionSeeking active feedback, usually for items under active developmentSeeking active feedback, usually for items under active development

   on May 5, 2026
9. MoralCode commented on May 7, 2026

   

   MoralCode

   on May 7, 2026

   Last edited by MoralCode

   ContributorAuthor

   More actions

   This issue essentially has three phases to the refactor:

   1. Create a new table in augur_operations for the forge instance data as described above Done
   2. ensure this new table is populated automatically at startup, especially if any keys are provided for the respective platforms
   3. detach existing foreign key references from the existing augur_data.platform table and find a new way to manage that
   4. Delete the old, no longer used, platform table, after verifying there are no remaining things using it.
10. MoralCode commented on Oct 3, 2026

    

    MoralCode

    on Oct 3, 2026

    Last edited by MoralCode

    ContributorAuthor

    More actions

    Where is this used?

    Github

    there are lines declaring platform_id = 1 in tasks/github:

    • messages.py
    • events.py
    • tasks.py (pull requests)
    • tasks.py (facade_github)
    • github_task_session.py

    existing uses of platform IDs (foreign key or no):
    issue_events:
    

    CollectOSS/collectoss/tasks/github/events.py

    Lines 192 to 193 in 8eb003b

    	issue_event_dicts.append(
    	extract_issue_event_data(event, issue_id, platform_id, repo_id,

    pr_events:
    

    CollectOSS/collectoss/tasks/github/events.py

    Lines 226 to 229 in 8eb003b

    	pr_event_dicts.append(
    	extract_pr_event_data(event, pull_request_id, int(event['issue']["id"]), platform_id, repo_id,
    	self._tool_source, self._tool_version, self._data_source)
    	)

    PR reviews:
    

    CollectOSS/collectoss/tasks/github/pull_requests/tasks.py

    Lines 531 to 534 in 8eb003b

    	if "cntrb_id" in review:
    	pr_review_dicts.append(
    	extract_needed_pr_review_data(review, pull_request_id, repo_id, platform_id, tool_version, data_source)
    	)

    Gitlab

    there are lines declaring platform_id = 2 in tasks.gitlab:

    • events_task.py
    • gitlab_task_session.py

    issue and PR events

    CollectOSS/collectoss/tasks/gitlab/events_task.py

    Lines 149 to 151 in 8eb003b

    	issue_event_dicts.append(
    	extract_gitlab_issue_event_data(event, issue_id, platform_id, repo_id,
    	tool_source, tool_version, data_source)

    CollectOSS/collectoss/tasks/gitlab/events_task.py

    Lines 199 to 202 in 8eb003b

    	mr_event_dicts.append(
    	extract_gitlab_mr_event_data(event, issue_id, platform_id, repo_id,
    	tool_source, tool_version, data_source)
    	)

    Generic

    5 instances of the string : platform_id, in data_parse.py (connecting the models to the DB schema)

    IssueEvent, PullRequestEvent, PullRequestReview and Message schemas make explicit foreign key references to the platform ID (which is really a type)

    What do we do about it?

    this is an expansion under what is fundamentally step 3 in the previous comment: detaching foreign key references

    1. Introduce enum for forge platform type to replace the magic numbers
    2. Create a non-enum structure in postgres for this type with a check constraint to limit the values to mirror the existing python enum so that we can consistently enforce constraints everywhere its referenced.
       - option 1: Domain. This is a bit of a bodge with maybe a postgres specific feature to avoid using an ENUM type, which cannot store values as integer and requires a more complex migration of every column thats currently FKed to the old platform table to translate its values to integers stored as strings.
       - option 2: https://michaelcho.me/article/using-python-enums-in-sqlalchemy-models/ (integer column) + check constraint check=f"VALUE IN ({', '.join(str(e.value) for e in OrderStatus)})",
    3. populate the forge_instances table at startup and update platform_type on operations.forge_instance to this new domain type
    4. replace the foreign key that refer to this platform id (all the ones listed in the prior section of this comment) in the DB with a foreign key to the forge_instances table - that is the closest match for what they are trying to be, and they preserve the domain association, while also keeping a reference to the type. This would not be possible if not for the coincidence that, for gh.com / gl.com, the type id and the instance id happen to match. As part of this step the enums need to be removed in the places where it was added in step 1 (anti magic number) and replaced with a number derived from the actual Instance entry identifier (which we can maybe invent out of thin air until we have proper passing of instance objects happening in collection)
    5. rename the columns that refer to this platform_id (which are actually platform_type) (can be deferred for later)
    6. All FKs to the old platform table should be gone, we can do a final check for usage and delete it.

    We will then be ready to:

    • populate forge_instance with the default values (github.com and gitlab.com) at startup for instances that would otherwise start with an empty forge_instances table
    • start relying on the forge_instances table as part of collection (key fetching, etc) and refactoring collection to be instance-aware
11. MoralCode

    added

    databaseRelated to the unifed data model/schemaRelated to the unifed data model/schema

    and removed

    discussionSeeking active feedback, usually for items under active developmentSeeking active feedback, usually for items under active development

    on Oct 3, 2026
12. MoralCode commented on Oct 4, 2026

    

    MoralCode

    on Oct 4, 2026

    Last edited by MoralCode

    ContributorAuthor

    More actions

    problem:
    I realize we cant swap the foreign keys (step 4) until we populate the forge instances table, but the FK swap is an alembic migration and the populating with defaults is done at app startup. and i cant block migrations until that startup script runs, and i dont reallllly want to put data/defaults inside an alembic migration

    Resolution: this might be a case where we have to allow alembic to be used to populate data. the alembic migrations are explicitly for existing instances moving over. the startup scripts are intended to simply populate data on first run
13. MoralCode

    marked this as blocking on Oct 4, 2026

    • Create general purpose CLI for checking rate limits #516
14. MoralCode commented on Oct 6, 2026

    

    MoralCode

    on Oct 6, 2026

    Last edited by MoralCode

    ContributorAuthor

    More actions

    Completion criteria:

    • create forge_instance table
    • add privacy columns to it (Add privacy policy columns to forge_instances table for assisting with compliance auditing #519 )
    • populate it with initial data (via migration - unavoidable)
    • add Reference to worker_oauth for api key
    • create migration to re-target the foreign keys to the platform table to point at the forge_instance table (this is what requires populating it with initial data)
    • delete the platform table
15. MoralCode

    marked this as blocked on Oct 6, 2026

    • Add privacy policy columns to forge_instances table for assisting with compliance auditing #519