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

fix: silverback build missing files during generate #151

Merged
merged 16 commits into from
Oct 28, 2024

Conversation

johnson2427
Copy link
Contributor

What I did

Added a check for requirements.txt and ape-config.yaml so silverback build doesn't fail if those files don't exist

fixes: #150

How I did it

Removed the lines pertaining to the requirements.txt and ape-config.yaml files and move them into a conditional within the build command.

How to verify it

python3 -m venv venv
pip install <this pr>
cd /to/a/silverback/directory/project
sivlerback build --generate

Check your dockerfile output in .silverback-images

mv requirements.txt req.txt

or if that file didn't exist

touch requirements.txt

silverback build --generate

Check your dockerfile output in .silverback-images and the requirements.txt lines should no longer be there.

Checklist

  • Passes all linting checks (pre-commit and CI jobs)
  • New test cases have been added and are passing
  • Documentation has been updated
  • PR title follows Conventional Commit standard (will be automatically included in the changelog)

fubuloubu
fubuloubu previously approved these changes Oct 24, 2024
silverback/_cli.py Outdated Show resolved Hide resolved
@johnson2427 johnson2427 force-pushed the fix/silverback-build-missing-files branch 4 times, most recently from db9fb0a to 76aae36 Compare October 25, 2024 20:50
silverback/_cli.py Outdated Show resolved Hide resolved
silverback/_cli.py Outdated Show resolved Hide resolved
silverback/_cli.py Outdated Show resolved Hide resolved
@fubuloubu
Copy link
Member

btw is there a way to tee the output of the build command so we see it in the output while it's building?

@johnson2427
Copy link
Contributor Author

btw is there a way to tee the output of the build command so we see it in the output while it's building?

That subprocess is supposed to do so with the stdout. I'll dig into this

@johnson2427 johnson2427 force-pushed the fix/silverback-build-missing-files branch from dc84413 to ea15206 Compare October 26, 2024 11:10
@johnson2427
Copy link
Contributor Author

@fubuloubu let me know how you'd like these changes to look. We're now getting a proper output (I was overwriting the PIPE to stderr from stdout before).

fubuloubu
fubuloubu previously approved these changes Oct 26, 2024
Copy link
Member

@fubuloubu fubuloubu left a comment

Choose a reason for hiding this comment

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

Looks perfect!

$ silverback build --generate
Generated ./.silverback-images/Dockerfile.paymaster
#0 building with "default" instance using docker driver

#1 [internal] load build definition from Dockerfile.paymaster
#1 transferring dockerfile: 445B done
#1 DONE 0.0s

#2 [internal] load metadata for ghcr.io/apeworx/silverback:stable
#2 DONE 0.0s

#3 [internal] load .dockerignore
#3 transferring context: 2B done
#3 DONE 0.0s

#4 [1/8] FROM ghcr.io/apeworx/silverback:stable
#4 DONE 0.0s

#5 [internal] load build context
#5 transferring context: 589B done
#5 DONE 0.0s

#6 [6/8] COPY ape-config.yaml .
#6 CACHED

#7 [5/8] RUN pip install --upgrade pip && pip install -r requirements.txt
#7 CACHED

#8 [4/8] COPY requirements.txt .
#8 CACHED

#9 [2/8] WORKDIR /app
#9 CACHED

#10 [3/8] RUN chown harambe:harambe /app
#10 CACHED

#11 [7/8] RUN ape plugins install -U .
#11 CACHED

#12 [8/8] COPY bots/paymaster.py /app/bot.py
#12 CACHED

#13 exporting to image
#13 exporting layers done
#13 writing image sha256:a8ac40894696c0fbf0c425525d749d2504ae6d3201e5359afd1275bbf0a2188c done
#13 naming to docker.io/library/paymaster:latest done
#13 DONE 0.0s

@fubuloubu
Copy link
Member

fubuloubu commented Oct 26, 2024

Actually @johnson2427, there is a utility function in ape that will hide your home directory path with $HOME, can you use that where it says Generated [...]/.silverback-images/Dockerfile.?

fubuloubu
fubuloubu previously approved these changes Oct 28, 2024
Copy link
Member

@fubuloubu fubuloubu left a comment

Choose a reason for hiding this comment

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

Looks much cleaner, some small updates I want to make

silverback/_cli.py Outdated Show resolved Hide resolved
silverback/_cli.py Outdated Show resolved Hide resolved
silverback/_build_utils.py Outdated Show resolved Hide resolved
silverback/_build_utils.py Outdated Show resolved Hide resolved
silverback/_build_utils.py Outdated Show resolved Hide resolved
silverback/_build_utils.py Outdated Show resolved Hide resolved
silverback/_build_utils.py Outdated Show resolved Hide resolved
@fubuloubu fubuloubu merged commit f7ab0e1 into main Oct 28, 2024
21 of 22 checks passed
@fubuloubu fubuloubu deleted the fix/silverback-build-missing-files branch October 28, 2024 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: silverback build fails if you do not have a requirements.txt or an ape-config.yaml
2 participants