Function to generate Flow types from edit data #1616

mwiencek · 2020-07-27T16:54:19Z

A SQL function, edit_data_type_info(), is added that generates "type info" for a single column of edit data.

A script, script/generate_edit_data_flow_type.js, is added which takes an edit type, selects edit_data_type_info() for every row of edit data for that type, and outputs a single merged Flow type representing the edit data overall for that type.

For example:

./script/generate_edit_data_flow_type.js --edit-type 92

Generates a Flow type for EDIT_RELATIONSHIP_DELETE.

The PROD_STANDBY database must be configured in DBDefs.pm to use this utility.

The underlying code used to build the Flow type may also be useful to generate Flow types for other JSON data, e.g. entities.json, so I've added script/generate_json_flow_type.js for this more general purpose too. Here's an example invoking that script:

cat entities.json | jq -c | script/generate_json_flow_type.js

(This expects the JSON object on a single line, so jq -c is used to compact the data first.)

I've committed the type generated for root/edit/details/RemoveRelationship.js as an example.

I think this will help with two things:

The SQL function by itself is useful to see all the different data formats an edit type has, which can make it easier to identify places we can clean up the data later.
The combined Flow type generated by edit_data_flow_type.js is useful for accessing the edit data safely (type-checked by Flow) in a React template. This isn't so important now, but in the future we'll likely want to move build_display_data etc. into JS.

mwiencek · 2020-08-26T19:27:11Z

Note that we can use ~~script/edit_data_flow_type.js~~ script/generate_json_flow_type.js to generate types for other JSON too, e.g. entities.json. I'd like to do that in another branch once this is merged. :)

reosarevok

LGTM but I didn't test

yvanzo

Successfully tested on production database with “Reorder relationships” edits (type ID = 99).

Would it make sense to have a wrapper script that uses PROD_STANDBY from DBDefs?

That would be nice to mention the command generating the added flow type in second commit too.

mwiencek · 2020-10-01T00:52:11Z

Would it make sense to have a wrapper script that uses PROD_STANDBY from DBDefs?

Added a wrapper script for that (and updated the commit message/PR description with instructions). It sets statement_timeout = 0 so you shouldn't get timeouts anymore.

Also added script/generate_json_flow_type.js to generate a Flow type from other JSON.

mwiencek · 2020-10-01T00:56:52Z

package.json

+    "pg": "8.3.3",
+    "pg-cursor": "2.3.3",


I'm also using these in my json-dumps rework.

yvanzo

./script/generate_edit_data_flow_type.js --edit-type 92

returned:

/musicbrainz-server/root/utility/generateFlowType.js:37
      (isEditDataTypeInfo ?? parent.isEditDataTypeInfo);
                           ^

SyntaxError: Unexpected token ?
    at Module._compile (internal/modules/cjs/loader.js:723:23)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/musicbrainz-server/script/generate_edit_data_flow_type.js:23:28)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)

using Node v10.19.0 currently installed in MusicBrainz Docker.

This will be used to connect directly to postgres from Node.js.

A SQL function, edit_data_type_info(), is added that generates "type info" for a single column of edit data. A script, script/generate_edit_data_flow_type.js, is added which takes an edit type, selects edit_data_type_info() for every row of edit data for that type, and outputs a single merged Flow type representing the edit data overall for that type. For example: ./script/generate_edit_data_flow_type.js --edit-type 92 Generates a Flow type for EDIT_RELATIONSHIP_DELETE. The PROD_STANDBY database must be configured in DBDefs.pm to use this utility. The underlying code used to build the Flow type may also be useful to generate Flow types for other JSON data, e.g. entities.json, so I've added script/generate_json_flow_type.js for this more general purpose too. Here's an example invoking that script: cat entities.json | jq -c | script/generate_json_flow_type.js (This expects the JSON object on a single line, so `jq -c` is used to compact the data first.)

Command used: ./script/generate_edit_data_flow_type.js --edit-type 92

yvanzo

Works fine now. Code looks good too. Maybe document scripts also in HACKING.md?

mwiencek · 2020-10-07T18:01:32Z

Maybe document scripts also in HACKING.md?

I wasn't sure where to put that so I made a general Flow section. Have a quick look. :)

yvanzo

Each time I approve this pull request, there are new additions, so I approve it again!

HACKING.md

mwiencek · 2020-10-07T18:34:27Z

OK, I'll commit the strict-local addition and merge this to save you from another approval. :)

mwiencek requested review from reosarevok and yvanzo August 26, 2020 19:21

reosarevok approved these changes Aug 28, 2020

View reviewed changes

mwiencek mentioned this pull request Sep 1, 2020

Remove dependency on lodash #1670

Merged

yvanzo approved these changes Sep 30, 2020

View reviewed changes

mwiencek force-pushed the edit-data-flow-types branch 3 times, most recently from f93d345 to 658b427 Compare October 1, 2020 00:49

mwiencek commented Oct 1, 2020

View reviewed changes

package.json

Comment on lines +51 to +52

"pg": "8.3.3",

"pg-cursor": "2.3.3",

Copy link

Member Author

mwiencek Oct 1, 2020

Choose a reason for hiding this comment

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

I'm also using these in my json-dumps rework.

yvanzo requested changes Oct 7, 2020

View reviewed changes

mwiencek force-pushed the edit-data-flow-types branch from 658b427 to c774398 Compare October 7, 2020 17:25

mwiencek requested a review from yvanzo October 7, 2020 17:28

mwiencek added 4 commits October 7, 2020 12:31

Output database configuration to DBDefs.js

a1da7ff

This will be used to connect directly to postgres from Node.js.

Add pg module

9ed21fe

This will be used to connect directly to postgres from Node.js.

Add Flow type for RemoveRelationship edit data

8402f87

Command used: ./script/generate_edit_data_flow_type.js --edit-type 92

mwiencek force-pushed the edit-data-flow-types branch from c774398 to 8402f87 Compare October 7, 2020 17:33

yvanzo approved these changes Oct 7, 2020

View reviewed changes

mwiencek force-pushed the edit-data-flow-types branch 2 times, most recently from 74ef207 to 319df9c Compare October 7, 2020 18:04

yvanzo approved these changes Oct 7, 2020

View reviewed changes

HACKING.md Outdated Show resolved Hide resolved

HACKING.md Show resolved Hide resolved

Document Flow in HACKING.md

583a592

mwiencek force-pushed the edit-data-flow-types branch from 319df9c to 583a592 Compare October 7, 2020 18:32

mwiencek merged commit 3fa61b7 into metabrainz:master Oct 7, 2020

mwiencek deleted the edit-data-flow-types branch October 7, 2020 18:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Function to generate Flow types from edit data #1616

Function to generate Flow types from edit data #1616

mwiencek commented Jul 27, 2020 •

edited

mwiencek commented Aug 26, 2020 •

edited

reosarevok left a comment

yvanzo left a comment

mwiencek commented Oct 1, 2020

mwiencek Oct 1, 2020

yvanzo left a comment

yvanzo left a comment

mwiencek commented Oct 7, 2020

yvanzo left a comment

mwiencek commented Oct 7, 2020

Function to generate Flow types from edit data #1616

Function to generate Flow types from edit data #1616

Conversation

mwiencek commented Jul 27, 2020 • edited

mwiencek commented Aug 26, 2020 • edited

reosarevok left a comment

Choose a reason for hiding this comment

yvanzo left a comment

Choose a reason for hiding this comment

mwiencek commented Oct 1, 2020

mwiencek Oct 1, 2020

Choose a reason for hiding this comment

yvanzo left a comment

Choose a reason for hiding this comment

yvanzo left a comment

Choose a reason for hiding this comment

mwiencek commented Oct 7, 2020

yvanzo left a comment

Choose a reason for hiding this comment

mwiencek commented Oct 7, 2020

mwiencek commented Jul 27, 2020 •

edited

mwiencek commented Aug 26, 2020 •

edited