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

Introduce remote table entity #4994

Merged
merged 3 commits into from
Apr 17, 2024
Merged

Conversation

thomtrp
Copy link
Contributor

@thomtrp thomtrp commented Apr 16, 2024

We will require remote table entity to map distant table name and local foreign table name.
Introducing the entity:

  • new source of truth to know if a table is sync or not
  • created synchronously at the same time as metadata and foreign table

Adding a few more changes:

  • exception rather than errors so the user can see these
  • pluralize library that will allow to stop adding Remote suffix on names

} from 'src/engine/metadata-modules/remote-server/remote-server.entity';

@Entity('remoteTable')
@ObjectType('RemoteTable')
Copy link
Member

Choose a reason for hiding this comment

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

You don't want ObjectType here, your graphql type is already defined in your dto


const localTableName = getRemoteTableLocalName(input.name);

this.validateTableNameDoesNotExists(
Copy link
Member

Choose a reason for hiding this comment

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

missing await here?

@@ -104,13 +106,76 @@ export class RemoteTableService {
throw new NotFoundException('Remote server does not exist');
}

const remoteTable = await this.createForeignTableAndMetadata(
if (!input.schema) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's save one query by checking this earlier! 😄

Copy link

github-actions bot commented Apr 16, 2024

TODOs/FIXMEs:

  • // TODO: This should be done in a transaction. Waiting for a global refactoring of transaction management.: packages/twenty-server/src/engine/metadata-modules/remote-server/remote-table/remote-table.service.ts

Generated by 🚫 dangerJS against 2971e62

@thomtrp thomtrp force-pushed the tt-introduce-remote-table-objects branch from 95601d3 to 2971e62 Compare April 17, 2024 07:39
@thomtrp thomtrp merged commit 6fa2aee into main Apr 17, 2024
14 of 16 checks passed
@thomtrp thomtrp deleted the tt-introduce-remote-table-objects branch April 17, 2024 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants