fix(ingestion/metabase) Fix for query template expressions and invalid URNs for Text Cards

[pulsar256]

Fixes #10380 Fixes #9767

Checklist

• [x] The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
• [x] Links to related issues (if applicable)
• [x] Tests for the changes have been added/updated (if applicable)
• [ ] Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
• [ ] For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

[pulsar256]

The code looks good, but this needs some tests

please take a look if the approach is ok. If it is fine, i will add another set of tests for the other issue and perhaps cover more variants of the templated queries.

[pulsar256]

@hsheth2 ready for round 2.

[pulsar256]

For tests for #9767, it would be sufficient to just add some unit tests that call strip_template_expressions

For #10380, since it's a fairly small change, it would also be better to modify the existing test instead of creating an entirely new one. The duplication will be harder to manage down the line

I have no problem undoing / removing the metabase mocked responses. I have considered monkey patching the existing mocked metabase responses I found it rather problematic given that I had no access to the metabase state to (re)generate those. In the end we want to test the ingestion with response payloads as close as possible to what metabase generates instead of hand crafted or patched responses. Hence I refactored the test suite to allow for side effect free (between the individual tests) usage of independent reponse mocks and I was looking into automating the capture process as well.

Do you want me to keep the refactored fixtures based approach for the response map in place or should we go back to the global state in the test suite?

[pulsar256]

(...)

Do you want me to keep the refactored fixtures based approach for the response map in place or should we go back to the global state in the test suite?

@hsheth2 ready for the next round

[pulsar256]

@hsheth2 thanks for the review. included both suggested changes, rebased.

[pulsar256]

@hsheth2 thanks, I think the workflows need another approval. Perhaps rebasing resets that?

[hsheth2]

@pulsar256 yup I believe it does. No need to rebase/merge master - we'll merge it once CI is green.