Skip to content

Conversation

@joehan
Copy link
Contributor

@joehan joehan commented Oct 15, 2024

Description

Adds support for data persistence between runs of the data conect emulator, as described in go/fdc-emu-import-api

Scenarios Tested

  • Set dataDir, ran a few CREATES, shut down the emulator, and verified they were still there on restart.
  • Tested out emulator:export and confirmed that it outputs a tarball database dump in the export directory.
  • Tested out --import and confirmed that the same data is there after import.
  • Curled the clearData endpoint and confirmed that it clears out data from all tables, and that the emulator still works and can write data afterwards.

Follow up work

  • Add a Clear Data admin endpoint that empties out this directory, and give VSCode users an easy button to trigger it.
  • E2E test for the data connect emulator that covers import/export

@joehan joehan changed the title MVP of persistence for the data connect emulator Import + export support for the data connect emulator Nov 15, 2024
@joehan joehan requested review from hlshen and yuchenshi November 15, 2024 18:56
return Emulators.DATACONNECT;
}

async clearData(): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

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

Can one still clear the DB even if running a separate postgres? I'd say yes, but any concerns?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good product q - I think it's reasonable to offer, but if they're connecting to a real DB that becomes a pretty scary big red button.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Chatted in local tooling sync - decision is that this is ok, as long as we have a confirmation dialogue in vsce

Copy link
Member

Choose a reason for hiding this comment

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

But given the code as it is written right now, this seems to noop if it's not using pglite.

I think it should either do what it says on the label, or fail with a clear error message (e.g. "unimplemented"), but not noop.

});

app.post(EmulatorHub.PATH_CLEAR_DATA_CONNECT, async (req, res) => {
// TODO: Sanity check that this is needed.
Copy link
Member

Choose a reason for hiding this comment

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

Probably needed

@joehan joehan requested review from hlshen and yuchenshi November 15, 2024 23:24
@joehan joehan enabled auto-merge (squash) November 20, 2024 00:00
@joehan joehan merged commit 47d4d1f into master Nov 20, 2024
44 of 45 checks passed
joehan added a commit that referenced this pull request Nov 21, 2024
joehan added a commit that referenced this pull request Nov 21, 2024
joehan added a commit that referenced this pull request Nov 21, 2024
joehan added a commit that referenced this pull request Nov 21, 2024
@joehan joehan deleted the jh-persistence-prototype branch November 21, 2024 18:28
joehan added a commit that referenced this pull request Nov 21, 2024
* Cleaning up some leftovers from #7836

* Close the conn too
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants