Hello World Sample

[zhongshixi]

Change Set

1. added an sample folder and introduces sample-as-code structure to demonstrate example usage with simple command for end user.

[zhongshixi]

@bretg here you go

[zhongshixi]

@shahinrahbariasl i would like to invite you to try out this initial sample and I am looking forward to your feedbacks

[zhongshixi]

@mkendall07

[zhongshixi]

@SyntaxNode , let me know if you have any opinion on it

[VeronikaSolovei9]

Hi @zhongshixi, thank you for the contribution! We discussed these changes with the team and have couple of suggestions. General idea is to have 2 options: set up PBS and run it

1. from sources
2. in Docker container.

This instruction is confusing because it will not work for the first option, for instance app.yaml should be pbs.yaml, stored responses should be placed under ./stored_responses/data/by_id/stored_responses and stored requests under ./stored_requests/data/by_id: "accounts", "stored_imps" and "stored_requests" directories. Instruction in this PR will work only for Docker container set up because it will copy files to the correct locations: https://github.com/prebid/prebid-server/pull/3326/files#:~:text=volumes%3A,bid%2Did.json It is confusing.

May you please add an instruction for the option 1 so users can distinct them and pick the one they need? It is partly described in the project root README.

We also think fetcher change should be a separate PR with unit tests, not a part of this PR. I also understand stored responses set up will not work without File System fetcher changes. Maybe fetcher PR should go first.

Question 1: why directory name is 001_banner? Question 2: do you think we can use simple docker run instead of docker compose up?

Proposed simplified steps to build and run PBS as a Docker container:

GOOS=linux GOARCH=amd64 go build -o ./build/prebid-server
docker build -f Dockerfile -t prebid-server .
docker run -d -p 8000:8000 prebid-server

Dockerfile:

FROM ubuntu:latest

RUN mkdir /app

COPY build/prebid-server /app

COPY static /app/static/
COPY stored_requests/data /app/stored_requests/data/
COPY stored_responses/data /app/stored_responses/data/
COPY pbs.yaml /app

WORKDIR /app

EXPOSE 8000
EXPOSE 6060
ENTRYPOINT ["/app/prebid-server"]

[zhongshixi]

@VeronikaSolovei9 Thank you for your reply, I will answer your point 1 by 1

We also think fetcher change should be a separate PR with unit tests, not a part of this PR. I also understand stored responses setup will not work without File System fetcher changes. Maybe fetcher PR should go first.

I agree with you. For this, I will make it a separate PR.

Question 1: why the directory name is 001_banner?

@bretg and I discussed the direction of the sample regarding the sample file structure. The goal of the sample to provide a progressive learning/demo experience, that's why there is 001 indicates the first demo a user should see, then banner means the demo is to demo a banner ad, if the next material is related to video ads, then I can add 002_video as another example.

Question 2: do you think we can use a simple docker run instead of a docker compose-up?

we could use docker run, but I have a different opinion of docker run is simple. In my opinion, the docker-compose is just docker with flags and values written in the YAML file and uses the one-line command docker-compose up to bring up everything. If I use bare docker, I have to construct lengthy docker build, and docker run commands with so many flags and values which is not simple from my perspective. docker-compose is a one-stop shop for everything needed here. Maybe there is a reason not to use docker-compose at all?

Proposed simplified steps to build and run PBS as a Docker container:

I will experiment with it and let you know. The proposed solution does look simple but does require the user to build the go binrary on the host first, let me see what I can do to make it a one-command procedure.

May you please add an instruction for option 1 so users can distinct them and pick the one they need?
It is partly described in the project root README.

sure, let me figure something out

[hhhjort]

This PR now also has some conflicts with master to resolve.

[bretg]

We got a question to Prebid Support email that this PR would help resolve.

@hhhjort and @guscarreon - please take a look and merge?

[VeronikaSolovei9]

Hi @bretg Some time ago I left couple of comments related to Dockerfile, FileSystem Fetcher and general set up description. Should they be still addressed?

[bretg]

Some time ago I left couple of comments

Could you address these comments yourself @VeronikaSolovei9 ? @zhongshixi has abandoned this PR but I still need it if at all possible because we get newbies who have trouble getting started.

[zhongshixi]

@bretg @VeronikaSolovei9 sorry will you give you new commit & update ASAP next week, i am not abandoning this PR

[zhongshixi]

going to push something tonight

[zhongshixi]

Added minor suggestion. Local testing looks good, instruction makes sense. I like the idea using existing Dockerfile to build an image, rather than inventing a new one (I suggested it earlier :))

UPD: Please keep in mind that File fetcher PR should be merged first.

i also tried to append the instruction for users who wants to use source build instead of Dockerfile, but it complicates things because i need to basically copy the required sample files to the source folders , worry about deleting them later instead of using volume mounting which to me is a very clean approach, i suggest we keep the sample simple and later add support for user who is source-build heavy

[zhongshixi]

cc @bsardo for visibility

[bsardo]

@zhongshixi we merged your local stored response fetching PR and introduced a conflict here. Can you resolve when you get a moment? Thanks!

[zhongshixi]

@bsardo updated PR to resolve the conflict, testing looks good

cc @bretg for visibility

[VeronikaSolovei9]

Hi @zhongshixi, thank you for the updates. I tested this PR again and this is what I see:

It worked before, but now something is not right

[zhongshixi]

@VeronikaSolovei9 looking into it now to see what happened

[VeronikaSolovei9]

This should fix it: https://github.com/prebid/prebid-server/pull/3634

[zhongshixi]

@VeronikaSolovei9 i saw the PR is closed - https://github.com/prebid/prebid-server/pull/3634#issuecomment-2070367928 which other place should i take a look for the corresponding fix?

[bsardo]

@zhongshixi I'll put up a PR shortly with the new fix and will ping you when it has been merged.

[bsardo]

@zhongshixi I merged in the fix (#3639) so hopefully that resolves any issues you were experiencing.

[zhongshixi]

going to merge from the latest master and try it again

[zhongshixi]

@bsardo thanks for the fix, it works again ( i have updated my branch to have the latest master branch code )

[zhongshixi]

@VeronikaSolovei9

docker compose build --no-cache

usually it is used to build the image completely from scratch, but you do not have to use it. The docker engine should be smart enough to understand if any layer of the image is changed.

take a look at the Dockerfile which contains COPY ./ ./ - so if you change any file in the repo that is not specified in .dockerignore when running docker-compose build , you should have a new image build.

maybe you are running a completely different private repo of the prebid server that is different from the open-sourced one which i do not have any visibility into. i can definitely help you if you could share more info about your environment

[zhongshixi]

@VeronikaSolovei9 no problem, thank you for all your feedbacks and review - excited about growing this tutorial module into a much more complete, matured, easy-to-use form in the future.