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

Include Redpanda assets in build artifact #790

Conversation

fabio-miguel
Copy link
Contributor

Fixes #762

This PR includes the redpanda.yaml.hbs file in the build artifact, addressing the issue where it was previously omitted. Non-TypeScript assets are organised into an assets folder in the redpanda module. The module's build script will copy these assets into the build folder.

Copy link

netlify bot commented Jun 30, 2024

Deploy Preview for testcontainers-node ready!

Name Link
🔨 Latest commit f806e25
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-node/deploys/668457a2ac241f00082b0a1d
😎 Deploy Preview https://deploy-preview-790--testcontainers-node.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@cristianrgreco cristianrgreco changed the title Include redpanda.yaml.hbs in build artifact #762 Include redpanda.yaml.hbs in build artifact Jun 30, 2024
@cristianrgreco cristianrgreco added bug Something isn't working patch Backward compatible bug fix labels Jun 30, 2024
@fabio-miguel fabio-miguel deleted the 762-Fix-redpanda.yaml.hbs-not-included-in-build-artifact branch July 1, 2024 12:03
@fabio-miguel fabio-miguel restored the 762-Fix-redpanda.yaml.hbs-not-included-in-build-artifact branch July 1, 2024 12:03
@fabio-miguel fabio-miguel reopened this Jul 1, 2024
@fabio-miguel
Copy link
Contributor Author

Fixes #790

The previous pull request failed the build due to code expecting to find the files at example: ⁠path.join(__dirname, "redpanda.yaml.hbs”) ⁠, but they were moved to an assets dir, which makes the tests fail. In this PR, location of these files were made consistent for the source files and the build. Therefore, asset files are always in an assets ⁠ dir. This perhaps makes it easier to add more files in future without worrying about build scripts.

Tests were run locally and passed.

Screenshot 2024-07-01 at 13 40 48

@fabio-miguel
Copy link
Contributor Author

Fixes #790

Apologies...

in the previous commit titled Fix build tests: Update asset paths & restore build script I put the ⁠ assets ⁠ directory in the ⁠ files ⁠ list of the ⁠ package.json ⁠ but this doesn’t affect which files are included in the TS ⁠ build ⁠. Running npm run build -w packages/modules/redpanda would mean the ⁠ build ⁠ directory doesn’t contain any assets. Also, I used path.join(__dirname, "../assets/bootstrap.yaml”). You used path.join to join together different directories in a way which works cross platform. ⁠ My use of ../assets/bootstrap.yaml ⁠ is not a valid path for example on Windows. I have adjusted this to path.join(__dirname, “assets”, “bootstrap.yaml.

Now, I have moved the assets directory to src in the redpanda module, updated paths in redpanda-container.ts, which work cross platform (as you originally intended), and modified the build script in the redpanda module to ensure consistency between source and build files by having the assets directory copied into the build.

@cristianrgreco cristianrgreco changed the title Include redpanda.yaml.hbs in build artifact Include Redpanda assets in build artifact Jul 3, 2024
@cristianrgreco cristianrgreco merged commit 5022cbc into testcontainers:main Jul 3, 2024
95 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working patch Backward compatible bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Seems like redpanda.yaml.hbs is not included in the build artifact
2 participants