Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dclid implementation to the CM connector and fixed a bug in the customVariables column #115

Closed
wants to merge 1 commit into from

Conversation

anaesqueda
Copy link
Collaborator

Add dclid implementation to the CM connector and fixed a bug where the customVariables (type/value) info was not retrieved from the table since it was not matching the regex for the filtered columns. Migrate CM API version to 4 since it will be deprecated in Feb 2023. Updated google-api-python-client to the latest version to support the CM API version 4. All tests passed. Tested CM connector and Customer Match connector.

…e customVariables (type/value) info was not retrieved from the table since it was not matching the regex for the filtered columns. Migrate CM API version to 4 since it will be deprecated in Feb 2023. Updated google-api-python-client to the latest version to support the CM API version 4. All tests passed.
@diogoaihara
Copy link
Contributor

Hi Ana,

I took a look and it seems nice. There is one case that I'm unsure about how it would behave (and I would like to check before giving a green light from my side): when having data sources that aren't bigquery. I will try to look at that tomorrow.

@caiotomazelli
Copy link
Collaborator

Hey Ana, thanks for the PR!

This looks good, but is pretty complex behavior. Would you mind adding a test that covers the customVariable nested use case so we make sure to not break it in the future?

@anaesqueda
Copy link
Collaborator Author

Hi team, thanks for the review! I have a meeting with Diogo today, we will check the implementation. Also, there is another approach that we have already discussed. Thanks!

@anaesqueda anaesqueda closed this Oct 28, 2022
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.

None yet

3 participants