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

Improve the ORM object names #806

Open
hanstrompert opened this issue Jan 29, 2025 · 3 comments
Open

Improve the ORM object names #806

hanstrompert opened this issue Jan 29, 2025 · 3 comments
Labels
breaking change code sprint ticket Berkeley visit enhancement Enhancement or extension of existing feature triage Issue that need to be triaged

Comments

@hanstrompert
Copy link
Member

Issue

The ORM is meant to be an object oriented abstraction on top of the underlying SQL database. For some reason, all ORM object names as defined in orchestrator/db/models.py end in Table:

FixedInputTable
ProcessStepTable
ProcessSubscriptionTable
ProcessTable
ProductBlockRelationTable
ProductBlockTable
ProductTable
ResourceTypeTable
SubscriptionCustomerDescriptionTable
SubscriptionInstanceRelationTable
SubscriptionInstanceTable
SubscriptionInstanceValueTable
SubscriptionMetadataTable
SubscriptionTable
WorkflowTable

when used in code, this does not provide the best readability experience:

def all_subscriptions() -> list[SubscriptionTable]:
    return SubscriptionTable.query.all()

in this example SubscriptionTable could better be replaced by Subscription.

Other object names can be confusing, like SubscriptionInstanceTable, that could have been named ProductBlockInstance, or maybe rename ProductBlock to ProductBlockType and rename SubscriptionInstanceTable to ProductBlock.

Proposal

Come up with a list of better object names and refactor the code base accordingly.

Advantage:

  • Improve readability of the code and the logic that it implements

Disadvantage:

  • Every user of the orchestrator-core has to refactor its own implementation as well
@hanstrompert hanstrompert added code sprint ticket Berkeley visit enhancement Enhancement or extension of existing feature triage Issue that need to be triaged labels Jan 29, 2025
@wouter1975
Copy link
Contributor

wouter1975 commented Jan 29, 2025

This ticket could also be combined by renaming the db tables and column, the minimal change could be:

Anyway the proposed new naming would as a minimum:

Table ok? New table name New table column names
subscription_instances no product_block_instances subscription_instance_id --> product_block_instance_id OK: subscription_id, product_block_id, label (can be removed)
subscription_instance_values no product_block_instance_values subscription_instance_id --> product_block_instance_id subscription_instance_value_id --> resource_type_instance_id OK: resource_type_id, value"
subscription_instance_relations no product_block_instance_relations ok
  • Main impact in de opensource code can be addressed by a find replace
  • In the custom code of other deployments besides SURF will require quite a bit of guidance (migration guide?)

An even better implementation is to change product_block to product_block_type and change product_block_instance to product_block

@pboers1988
Copy link
Member

There are two ways we could go about this. I feel we could rename everything to be Product centric or Subscription centric. I will give the example below:

Product centric

Old New
Product Product
ProductBlock ProductBlock
ResourceType ProductBlockAttribute
FixedInput ProductFixedAttribute
Subscription ProductInstance
SubscriptionInstance ProductBlockInstance
SubscriptionInstanceValue ProductBlockAttributeValue
SubscriptionInstanceRelations ProductBlockInstanceRelations
ProductBlockRelations ProductBlockRelations
ProductBlockResourceTypes ProductBlockProductBlockAttribute

Subscription centric

Old New
Product SubscriptionDefinition
ProductBlock SubscriptionBlockDefinition
ResourceType SubscriptionBlockAttributeDefinition
FixedInput SubscriptionFixedAttributeDefinition
Subscription Subscription
SubscriptionInstance SubscriptionBlock
SubscriptionInstanceValue SubscriptionBlockAttribute
SubscriptionInstanceRelations SubscriptionBlockRelations
ProductBlockRelations SubscriptionBlockRelationsDefinition
ProductBlockResourceTypes SubscriptionBlockSubscriptionBlockAttributes

@wouter1975
Copy link
Contributor

wouter1975 commented Feb 11, 2025

Old New
Product Product -- no change
FixedInput ProductFixedAttribute
ProductBlock ProductBlock -- no change
ResourceType ProductBlockAttribute
ProductBlockRelations ProductBlockRelations -- no change
ProductBlockResourceTypes ProductBlockAttributes
Subscription Subscription -- no change
SubscriptionInstance SubscriptionProductBlock
SubscriptionInstanceValue SubscriptionProductBlockAttribute
SubscriptionInstanceRelations SubscriptionProductBlockRelations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change code sprint ticket Berkeley visit enhancement Enhancement or extension of existing feature triage Issue that need to be triaged
Projects
None yet
Development

No branches or pull requests

3 participants