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

New Adapter: Generic #3743

Closed
wants to merge 1 commit into from
Closed

New Adapter: Generic #3743

wants to merge 1 commit into from

Conversation

przemkaczmarek
Copy link
Collaborator

No description provided.

Copy link

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, a5b9255

generic

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/generic/generic.go:27:	Builder		0.0%
github.com/prebid/prebid-server/v2/adapters/generic/generic.go:34:	MakeRequests	0.0%
github.com/prebid/prebid-server/v2/adapters/generic/generic.go:50:	MakeBids	0.0%
github.com/prebid/prebid-server/v2/adapters/generic/generic.go:96:	getBidType	0.0%
total:									(statements)	0.0%

@bsardo bsardo changed the title new: Adapter Generic New Adapter: Generic Jun 11, 2024
@bsardo bsardo assigned bsardo and unassigned przemkaczmarek Jun 13, 2024
Copy link
Contributor

@SyntaxNode SyntaxNode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for implementing the generic adapter. This will be more complicated than other adapter ports due to the special case nature of the generic adapter. We'll likely want to talk through some of these comments. I'm out for the June 19 meeting though.

"minLength": 1
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's invalid in PBS-Go for aliases to specify a different json schema. Does PBS-Java allow generic aliases to specify their own schema?

- native
- audio
supported-vendors:
vendor-id: 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PBS-Java config is not valid for PBS-Go. This should break out into one file per alias of the generic adapter.

IMHO the root generic adapter is a special case which should not have a generic.yaml file, which likely requires extra logic.

}

type genericBidExt struct {
VideoCreativeInfo *genericBidExtVideo `json:"video,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do bidders return these fields today in PBS-Java?


}
}
return bidType
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the approach used by PBS-Java? It's an antipattern which is all too common in adapters. IMHO we should prefer MType, then perhaps the prebid metadata extension, and maybe finally fall back to this approach,, in which case audio needs to be supported here too.

@bsardo
Copy link
Collaborator

bsardo commented Nov 5, 2024

Closing. We will revisit at some point and will need to discuss as this is quite involved.

@bsardo bsardo closed this Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants