MaterializeInc / demos

Demos of Materialize, the operational data warehouse.
https://materialize.com
Apache License 2.0
50 stars 8 forks source link

demos: refactor `ecommerce` demo #21

Closed morsapaes closed 2 years ago

morsapaes commented 2 years ago

The existing ecommerce demo doesn't have a clear separation of concerns for certain components (e.g. deployment of the Debezium connector), which makes it hard to parse for users and causes CI tests to be spotty. This PR shuffles things around to address that. Also fixes #14, though we could still strip it further down by using debezium/connect-base instead of debezium/connect.

~Just need to figure out how to add a test for ecommerce-redpanda, now that it's embedded in the ecommerce directory.~ One of the materialized views in ecommerce is also spitting out weird results (need to double check if it's pre-existing or related to bumping the materialized version somehow, since nothing else changed), but putting it out for review!

morsapaes commented 2 years ago

@benesch, turns out that upload-artifact@v2 doesn't munch through special characters, and that ecommerce/redpanda is interpreted as a directory also for the .sh test file (even escaping the slash). This is a bit gross, but:

https://github.com/MaterializeInc/demos/pull/21/files#diff-7829468e86c1cc5d5133195b5cb48e1ff6c75e3e9203777f6b2e379d9e4882b3R43-R54

The lint check also complained, so I tweaked it to check up to two levels up:

https://github.com/MaterializeInc/demos/pull/21/files#diff-4416b1b334979f71ea4111156b2048d51607675a3a03b833b82c365cba1b7afdR122

All of this feels kind of 👷‍♀️🛠️, so let me know if the changes make sense or if there's a more elegant way to go around this.

benesch commented 2 years ago

I guess the question I'd be asking is whether it's worth the trouble to nest the demo directories. If it is, then I think you're stuck doing something like this!

It might be a little clearer if you called the nested demo ecommerce/redpanda? Then you could declare that the name of the demo is its relative path with all slashes replaced with hyphens. The Python computation of the demo name becomes str(path.parent).replace("/", "-") and the shell computation of the demo name becomes echo "DEMO=$(echo ${{ matrix.demo }} | tr / -)" >> $GITHUB_ENV.

No strong feels though!

morsapaes commented 2 years ago

I guess the question I'd be asking is whether it's worth the trouble to nest the demo directories. If it is, then I think you're stuck doing something like this!

Yeah, I was prematurely worrying about how the repo might grow unruly as we add more demos (via not being able to unsee this vs. this). Thanks for keeping me grounded! :beneschzers:


@ruf-io, there's something weird going on with the item_pageviews view also in the original repo, so I'll merge this as is to unblock the last mile and get the repo live.