feat(ads): dynamic Google Ads enum introspection helper#33
Open
illia-sapryga wants to merge 2 commits intokLOsk:mainfrom
Open
feat(ads): dynamic Google Ads enum introspection helper#33illia-sapryga wants to merge 2 commits intokLOsk:mainfrom
illia-sapryga wants to merge 2 commits intokLOsk:mainfrom
Conversation
Adds adloop.ads.enums.enum_names() — pulls valid enum member names
straight from the google-ads SDK at the API version we're pinned to
(see adloop.ads.client.GOOGLE_ADS_API_VERSION).
Drops the need to hand-maintain parallel lists like:
_VALID_CONVERSION_ACTION_TYPES = {"AD_CALL", "WEBSITE_CALL", ...}
— which otherwise drift every time the SDK or API version updates.
The helper is module-cached (functools.lru_cache) so the no-auth
GoogleAdsClient used for introspection is built once per process,
and every enum_names() call after the first is essentially free.
UNSPECIFIED + UNKNOWN sentinels are dropped by default since they
are protobuf zero-values that should never appear in user input.
Pure addition. No existing validators changed in this PR; downstream
PRs (conversion-action tools, in-place asset updates, promotion
helper refactors) will switch their hardcoded enum sets to enum_names()
calls in their own commits.
12 unit tests cover:
- enum_names returns a frozenset
- UNSPECIFIED + UNKNOWN are excluded by default and includable on opt-in
- Critical members (AD_CALL, WEBSITE_CALL, GOOGLE_SEARCH_ATTRIBUTION_DATA_DRIVEN,
USE_RESOURCE_LEVEL_CALL_CONVERSION_ACTION, BLACK_FRIDAY) are present
- ConversionActionCountingTypeEnum returns exactly {ONE_PER_CLICK, MANY_PER_CLICK}
- Unknown enum names raise AttributeError
- LRU cache returns the same frozenset instance on repeat calls
- The introspection client is memoized
6 tasks
The TestModulesUseDynamicEnums class was importing adloop.ads.conversion_actions and asserting on adloop.ads.write._VALID_PROMOTION_OCCASIONS — modules / constants that don't exist in this PR. They live in the follow-up PR (kLOsk#34, feat/asset-and-conversion-tools) and should be tested there. Branch A is intentionally minimal: the helper + helper-only unit tests. CI now passes — 12/12 tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
adloop.ads.enums.enum_names()— a tiny helper that pulls valid enum member names straight from thegoogle-adsSDK at the API version we're pinned to.The motivation: AdLoop currently hand-maintains parallel lists like
inside
ads/write.pyand elsewhere. Those lists drift the moment the SDK or API version updates — the SDK adds new enum members, our hardcoded set doesn't. Result: AdLoop's validators reject API-valid input, OR (worse) accept input the API would reject because the deny-list logic was outdated.This PR introduces a single helper that callers use like:
The result is a
frozensetof enum member names (e.g.{"AD_CALL", "WEBSITE_CALL", ...}) with the protobufUNSPECIFIEDandUNKNOWNsentinels dropped by default. The helper isfunctools.lru_cache'd so the no-authGoogleAdsClientused for introspection is constructed once per process — everyenum_names()call after the first is essentially free.Why no other modules are touched in this PR
This PR is intentionally scoped to just the helper. Downstream PRs (conversion-action management, in-place asset updates, promotion helper refactors) will switch their hardcoded enum sets to
enum_names()calls in their own commits.What this catches
During development, swapping
_VALID_CONVERSION_ACTION_TYPEStoenum_names("ConversionActionTypeEnum")revealed that the hardcoded list was missing 29 of 40 valid types in the SDK — includingCLICK_TO_CALL,FIREBASE_*,ANDROID_APP_PRE_REGISTRATION, etc. AdLoop would have rejected legitimate user input for any of those.Test plan
enum_names()returns afrozensetUNSPECIFIED+UNKNOWNare excluded by defaultexclude_unspecified=Falseopt-in includes the sentinelsConversionActionCountingTypeEnumreturns exactly{ONE_PER_CLICK, MANY_PER_CLICK}AttributeErrorfrozensetinstance on repeat callsuv run pytest→ 196/196 tests pass (184 baseline + 12 new)