update dev fixes to stage #627
Conversation
Feature/bidding and auction
Documentation Updates: The bidding-and-auction.md file has been updated to clarify the demo's goals, assumptions, and key exclusions. The steps have been revised to provide more precise instructions for users. Console Logging: Console log messages in ssp-a, ssp-x, and ssp-y components have been modified to provide more context about the auction configurations. Code Cleanup: Unnecessary code related to encoding requests has been removed from ssp-y/construct-component-auction.ts.
* [docker-compose] ad network configuration Setting network configuration to allow integration with bidding and auction demo. Privacy Sandbox Demos will use 172.16.128.0/24 range to not collide with Bidding and Auction services with fixed ips on 172.16.0.0/24 * package.json scripts - update start command to be able to refresh a specific service. use `npm run start -- [service-name]` - update stop command to remove volumes - update build command to build without cache * docker-compose : fix race condition on ad-tech services ad-tech services are using the same codebase and were all mounting the local ./services/ad-tech folder causing race conditions. Often one of the ad-tech services would not start properly. as a temporary workaround we are removing the filesystem mounted volume.
* add xxx-news services in firebase for Multi Touch Attribution Demo * multi-touch attribution demo doc : update link * fix firebase_deploy.sh
* demo docs fixes : - fix issue #624 - fix issue #623 - fix issue #622 - fix issue #621 Documentation Fixes: The PR resolves multiple documentation issues by fixing relative file paths and URLs in the markdown files. It also updates image links to use relative paths instead of absolute URLs. Docusaurus Configuration: The Docusaurus configuration is updated to throw an exception when Markdown links are broken, ensuring link integrity. The trailingSlash option is enabled to add trailing slashes to URLs. Nginx Configuration: The Nginx configuration is strengthened by upgrading Nginx to version 1.27.5-alpine and adding directives for security and optimization. The configuration file is now directly copied to /etc/nginx/nginx.conf instead of using a template. Docker Compose Update: The docker-compose.yml file is updated to use the newer Nginx image version 1.27.5-alpine. Markdown file paths are preferred. see https://docusaurus.io/docs/next/markdown-features/links updated docusaurus config to throw an exception when Markdown links are broken * [home] strenghten nginx configuration upgrade nginx add nginx.conf directives for security and optimization * fix docs link to full sized image hosted on Github
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @sinnew, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request updates the dev fixes to the stage environment. It includes changes to environment files, docker-compose configuration, firebase settings, package.json scripts, and documentation updates related to bidding and auction demos. The changes primarily focus on adding new services, updating dependencies, modifying deployment scripts, and enhancing the bidding and auction demo documentation.
Highlights
- Service Additions: Adds new services ('moto-news', 'soccer-news', 'gardening-news') to the environment configurations and firebase settings.
- Dependency Updates: Updates the nginx image version in the docker-compose file from 1.22.1-alpine to 1.27.5-alpine.
- Script Modifications: Modifies the firebase deployment script to remove checks for Firebase Hosting sites and adds a command to list hosting sites.
- Documentation Enhancements: Updates the bidding and auction demo documentation with improved explanations, diagrams, and instructions.
- Bidding and Auction Logic: Modifies the bidding and auction logic for SSP-A, SSP-X and SSP-Y to include on-device auction configurations and mixed mode auctions.
Changelog
Click here to see the changelog
- .env
- Adds 'moto-news', 'soccer-news', and 'gardening-news' to the list of services.
- .firebaserc.template
- Adds 'news', 'shop', 'travel', 'moto-news', 'soccer-news', and 'gardening-news' to the firebase hosting configuration.
- Removes duplicate 'shop', 'news', and 'travel' entries.
- cicd/.env.dev
- Adds 'moto-news', 'soccer-news', and 'gardening-news' to the list of services.
- Updates the URIs for soccer-news and gardening-news to include '.web.app'.
- cicd/.env.prod
- Adds 'moto-news', 'soccer-news', and 'gardening-news' to the list of services.
- docker-compose.yml
- Removes volume mounts for './services/ad-tech' for multiple services (dsp, dsp-a, dsp-b, dsp-x, dsp-y, ssp, ssp-a, ssp-b, ssp-x, ssp-y, ad-server).
- Updates the nginx image version to 1.27.5-alpine.
- Adds an ipam configuration for the adnetwork.
- firebase.json
- Renames firebase targets from 'topics-motorcycles', 'topics-soccer', and 'topics-gardening' to 'moto-news', 'soccer-news', and 'gardening-news' respectively.
- package.json
- Updates the 'start', 'stop', and 'build' scripts for both docker and podman to include --build, --force-recreate, --volumes, --no-cache, and --timeout options.
- scripts/firebase_deploy.sh
- Removes the loop that checks all Firebase Hosting sites and purges the cache.
- Adds a command to list Firebase Hosting sites.
- services/ad-tech/src/public/js/ssp/usecase/bidding-and-auction/ssp-a/construct-component-auction.ts
- Adds '[on-device]' to the console log message for component auction config.
- services/ad-tech/src/public/js/ssp/usecase/bidding-and-auction/ssp-x/construct-component-auction.ts
- Changes the protectedAudienceAuctionResult to be a Uint8Array.
- Adds '[B&A only]' to the console log message for component auction config.
- services/ad-tech/src/public/js/ssp/usecase/bidding-and-auction/ssp-y/construct-component-auction.ts
- Removes the #encodeRequest method.
- Changes the protectedAudienceAuctionResult to not be a Uint8Array.
- Adds '[B&A mixed mode]' to the console log message for component auction config.
- services/ad-tech/src/routes/ssp/usecase/bidding-and-auction/bidding-and-auction-router.ts
- Updates the bidUrl for DSP_A_ORIGIN to '/dsp/contextual-bid-ba'.
- services/home/Dockerfile
- Updates the nginx base image to 1.27.5-alpine.
- Copies nginx.conf instead of using a template.
- Adds an EXPOSE directive for port 8080.
- services/home/docs/demos/bidding-and-auction.md
- Updates the goals and assumptions of the bidding and auction demo.
- Replaces a local image link with a github link.
- Adds instructions to delete the docker network before running
npm run start. - Removes
BIDDING_SIGNALS_FETCH_MODEfrom the start_bfe commands. - Updates the steps to navigate to the shop and news sites.
- Adds details about the SSP-Y sub-component auction for B&A.
- Updates the seller details section with more specific information about the news router and ad-tag file.
- Adds more details about the unified auction route and runProtectedAudienceAuction function.
- services/home/docs/demos/instream-video-ad.md
- Updates the link to the sequential auction setup demo.
- Updates links to the bidding logic and video-ad-frame.js files.
- services/home/docs/demos/multi-piece-ad.md
- Updates the link to the sequential auction setup demo.
- services/home/docs/demos/multi-touch-conversion-attribution.md
- Updates the link to the DSP service report visualization page.
- services/home/docs/demos/publisher-ad-quality-req.md
- Updates the link to the publisher-ad-quality-req-flow.png file.
- Updates the link to the retargeting-remarketing demo.
- Updates the link to the
window.PSDemo.PAGE_ADS_CONFIGfile. - Updates the link to the
ad-server-tag.jsfile. - Updates the link to the sequential auction setup demo.
- services/home/docs/demos/sequential-auction-setup.md
- Updates the link to the retargeting-remarketing demo.
- Updates the link to the
run-sequential-ad-auction.jsfile.
- services/home/docs/intro.md
- Updates the links to the bidding-and-auction and multi-touch-conversion-attribution demos.
- services/home/docusaurus.config.js
- Adds
trailingSlash: trueto the config. - Changes
onBrokenMarkdownLinkstothrow.
- Adds
- services/home/nginx.conf
- Replaces the simple server block with a full nginx configuration file for better control over settings like gzip, caching, and security headers.
- services/news/.prettierignore
- Removes *.js from the .prettierignore file.
- services/news/src/views/bidding-and-auction.ejs
- Minor formatting changes.
- services/news/src/views/components/aside.ejs
- Minor formatting changes.
- services/news/src/views/components/display-ads.ejs
- Removes the display-ads component.
- services/news/src/views/components/footer.ejs
- Minor formatting changes.
- services/news/src/views/components/header.ejs
- Minor formatting changes.
- services/news/src/views/components/video-ad.ejs
- Minor formatting changes.
- services/news/src/views/example-private-aggregation.ejs
- Minor formatting changes.
- services/news/src/views/index.ejs
- Removes the display-ads component.
- Minor formatting changes.
- services/news/src/views/mmt-basic-reach-mmt.ejs
- Minor formatting changes.
- services/news/src/views/mmt-multi-touch-attribution.ejs
- Minor formatting changes.
- services/news/src/views/mmt-single-touch-attribution-html.ejs
- Minor formatting changes.
- services/news/src/views/mmt-single-touch-attribution-js.ejs
- Minor formatting changes.
- services/news/src/views/pa-display-ad-with-deals.ejs
- Updates the code for the ad unit.
- Minor formatting changes.
- services/news/src/views/pa-fenced-frame-display-ad.ejs
- Updates the code for the ad unit.
- Minor formatting changes.
- services/news/src/views/pa-iframe-display-ad.ejs
- Updates the code for the ad unit.
- Minor formatting changes.
- services/news/src/views/pa-iframe-multi-piece-ad.ejs
- Updates the code for the ad unit.
- Minor formatting changes.
- services/news/src/views/pa-iframe-video-ad.ejs
- Updates the code for the ad unit.
- Minor formatting changes.
- services/shop/.prettierignore
- Removes *.js from the .prettierignore file.
- services/shop/src/views/cart-item.ejs
- Minor formatting changes.
- services/shop/src/views/cart.ejs
- Minor formatting changes.
- services/shop/src/views/checkout.ejs
- Minor formatting changes.
- services/shop/src/views/index.ejs
- Minor formatting changes.
- services/shop/src/views/item.ejs
- Minor formatting changes.
- services/travel/src/views/index.ejs
- Minor formatting changes.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A stage for code, where fixes align,
Dev's dance to stage, a polished design.
New services bloom,
Dispelling all gloom,
To stage, the code will shine.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request introduces several updates and bug fixes across various files, primarily focusing on configuration adjustments, dependency updates, and documentation improvements. The addition of new services and corresponding configurations seems well-organized. However, some areas require closer attention, particularly regarding the removal of volume mounts in docker-compose.yml and the accuracy of documentation links.
Summary of Findings
- Missing Volume Mounts: The removal of volume mounts for
./services/ad-techindocker-compose.ymlmay impact the functionality of ad-tech services. Verify that this change is intentional and that the services can still access necessary resources. - Inaccurate Documentation Links: Several documentation links in
services/home/docs/demos/bidding-and-auction.mdpoint to specific commit hashes or thedevbranch, which may become outdated or unstable. Consider using more stable links to themainbranch or permanent URLs. - Inconsistent Nginx Configuration: The
services/home/Dockerfilereplaces the template-based Nginx configuration with a direct copy ofnginx.conf. Ensure that the new configuration is fully compatible with the application's requirements and that environment variables are correctly handled.
Merge Readiness
The pull request includes several important updates and bug fixes. However, before merging, it's crucial to verify the removal of volume mounts in docker-compose.yml and update the documentation links to ensure stability and accuracy. Additionally, the change in Nginx configuration should be thoroughly tested to prevent any compatibility issues. I am unable to approve this pull request, and recommend that others review and approve this code before merging. At a minimum, the high severity issues should be addressed before merging.
Allows env vars (PORT) in nginx conf
Merge pull request #616 from privacysandbox/feature/bidding-and-auction
Feature/bidding and auction (https://github.com/privacysandbox/privacy-sandbox-demos/pull/619[)](https://github.com/privacysandbox/privacy-sandbox-demos/commit/8a34d34e6dae36161c4e469f3edb51e540414841)
Bugfix/mmt-multi-touch (https://github.com/privacysandbox/privacy-sandbox-demos/pull/620[)](https://github.com/privacysandbox/privacy-sandbox-demos/commit/ff68148e0987979ecdac2f0183b9ca2a1b847bcc)
Bugfix/home docs (https://github.com/privacysandbox/privacy-sandbox-demos/pull/626[)](https://github.com/privacysandbox/privacy-sandbox-demos/commit/01d3479a4c6a2b4453bbdb0632aa48b5b79eb284)
Dockerfile Modification
Nginx Configuration Update
[home] strenghten nginx configuration
upgrade nginx
add nginx.conf directives for security and optimization (nginx was using the default config so far)
docusaurus config s #626