Skip to content

Conversation

erindru
Copy link

@erindru erindru commented Aug 13, 2025

I started reviewing TobikoData#4751 again but quickly realised there were some more fundamental problems. Namely, a lot of things were implemented that did not need to exist and I figured that there would be a lot of back-and-forth that would ultimately just end up getting fed into Claude and not actually addressing the underlying problems.

So this PR does the following:

  • Strip out a bunch of the vibe code in favour of re-using existing constructs
  • Change CatalogSupport.FULL_SUPPORT to CatalogSupport.REQUIRES_SET_CATALOG because it seems far more operations than not need the catalog to be set, including querying INFORMATION_SCHEMA
  • Refactor the HTTP code into a separate class to keep it contained and remove some of the boilerplate
  • Remove unnecessary checks like "ensure schema exists before creating view or table" - SQLMesh already does this
  • Simplify connection factory override in FabricEngineAdapter constructor
  • Fix unit tests

The result passes our integration test suite but I didn't run it against an actual project.

@fresioAS if you have something locally do you mind doublechecking it?

@fresioAS
Copy link
Owner

Thank you @erindru !

I will run this on my live project tomorrow and report back!

@fresioAS fresioAS merged commit d9729a1 into fresioAS:add_fabric_warehouse Aug 13, 2025
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