Materialize: CI Metabase job is not reproducible

Created on 20 Feb 2020  ·  7Comments  ·  Source: MaterializeInc/materialize

@JLDLaughlin apologies for not catching this in review of https://github.com/MaterializeInc/materialize/pull/2065, but I just noticed that the CI job depends on MaterializeInc/metabase:latest.

From experience this will cause problems, because the CI pipeline for this repository is no longer deterministic. If someone pushes a bad build to MaterializeInc/metabase:latest, suddenly this CI pipeline will start failing. This is part of the reason that I'm such a big proponent of the monorepo: you can't have this kind of problem if everything is contained in one repository.

If https://github.com/MaterializeInc/metabase-materialize-driver were a long term project, I'd say we should merge it into this repository. But since it's (I think) just a short term solution while we work on better pgcompat, then I think it makes sense to keep it as a separate repository—but we still need to figure out the reproducibility aspect.

One option is to pin to a more specific metabase image, like MaterializeInc/metabase:SOMEDATE, and then make a pact to never modify these Docker tags. The downside is that you'd need to remember to periodically update the tag as changes occur to metabase-materialize-driver—but that's just sort of the cost to having multiple repos, IMO.

Thoughts?

/cc @quodlibetor

C-bug

All 7 comments

I think that we're likely to always have a specific metabase driver, it is the hook for customization in metabase. The part that we want to get rid of is MaterializeInc/pgjdbc, which is indeed just a hack until we have better pg compat.

I would be fine creating a compat or drivers directory in MaterializeInc/materialize and merging our metabase plugin (which is confusingly named by the standard metabase naming scheme and called a driver 🤷‍♀️)

we do have tagged versions of the materialize/metabase docker image, though, so we could reasonably change the version in materialize to materialize/metabase:v0.0.5 -- in particular I don't think it wants to have the same versioning scheme as materialize itself, but it _is_ a public project that we want people to use, so the build system described in #2077 would need to be able to handle that. IMO that's fine, but should be at least kind of considered.

I would be fine creating a compat or drivers directory in MaterializeInc/materialize and merging our metabase plugin (which is confusingly named by the standard metabase naming scheme and called a driver 🤷‍♀️)

Ah, ok. I do have a small preference for waiting until we don't need to merge all of pgjdbc into the main repo.

Ideally, we want to submit a PR upstream to Metabase to include our metabase-materialize-driver as part of the core Metabase repository (outlined here). Once we do that, we should be able to remove the metabase-materialize-driver repository altogether. So in my mind, our driver repo is temporary and shouldn't be merged into MaterializeInc/materialize.

^ does this seem reasonable to you @quodlibetor ?

In the meantime, I'm happy to hardcode a metabase version number (v0.0.5 as @quodlibetor mentioned above) into the failing CI test.

👍👍👍 sounds great to me, FWIW. It seems correct to me that our CI would hardcode some upstream Metabase version to test compatibility with, and that we'd periodically update that hardcoded version.

Oh that's fine! yeah I'd be happy to merge our driver upstream, I didn't realize quite how many metabase shipped with ootb. That seems strictly superior.

Awesome! Our PR blocker still seems to be moving from our hacky pgjdbc -> a real mzjdbc. (Ticket here).

In the meantime: #2094

Was this page helpful?
0 / 5 - 0 ratings