Skip to content
This repository has been archived by the owner on Sep 26, 2022. It is now read-only.

Remove opengraph now that UI doesn't depend on it #67

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jshearer
Copy link
Contributor

@jshearer jshearer commented Sep 12, 2022

Blocked on estuary/ui#319, since otherwise if we merge this the UI will break because it's asking for a field that won't exist

@jgraettinger is it... okay to just nuke all of this opengraph stuff? Is it being used for anything else?


This change is Reviewable

Copy link
Member

@jgraettinger jgraettinger left a comment

Choose a reason for hiding this comment

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

Big picture looks right, a couple of comments below.

Can you include as a comment or within commit message the details on how you've tested this? Thanks

@@ -14,181 +14,170 @@ declare
connector_id flowid;
begin
Copy link
Member

Choose a reason for hiding this comment

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

There appear to be deeper changes here than just updating the associated metadata fields?

In any case perhaps we can just kill this seed_connectors.sql. We have local/install-connector.sh for local development, and it's not a goal that we have source-control for every connector installed in the production DB, so I don't believe it serves a purpose anymore.

alter view live_specs_ext owner to authenticated;

-- Extended view of user draft specifications.
create view draft_specs_ext as
Copy link
Member

Choose a reason for hiding this comment

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

Q: what changed about draft_specs_ext, that it must be migrated?


ALTER TABLE IF EXISTS public.connectors DROP COLUMN IF EXISTS open_graph_patch;

DROP FUNCTION IF EXISTS public.generate_opengraph_value(opengraph_raw jsonb, opengraph_patch jsonb, field text);
Copy link
Member

Choose a reason for hiding this comment

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

Wrap in begin;, commit; ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants