-
Notifications
You must be signed in to change notification settings - Fork 0
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
[maykinmedia/open-api-framework#96] Add quick-start.yml #9
base: main
Are you sure you want to change the base?
Conversation
c518126
to
2090134
Compare
a240514
to
c4f1038
Compare
steps: | ||
- uses: actions/checkout@v4 | ||
- name: Start docker containers | ||
run: docker compose up -d --build || ( docker compose logs >&2 && exit 1; ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my information: is the explicit exit 1
needed, wouldn't the build/bringing the containers up failing cause a non zero exit code by itself? In any case I think it's fine to use this to make sure the job fails if there are any errors
done | ||
- name: Load fixtures | ||
if: ${{ inputs.fixtures != '' }} | ||
run: docker compose exec -T web src/manage.py loaddata ${{ inputs.fixtures }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it work if you pass multiple fixture names to fixtures
?
SUPERUSER="${{ inputs.superuser || 'admin' }}" | ||
WEB_SERVICE="${{ inputs.web_service || 'web' }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're not testing logging in in this job, I don't think we have to make these variable, but as long as it's not required to pass them it's fine by me to leave this as is
docker compose exec -T $WEB_SERVICE src/manage.py createsuperuser --username $SUPERUSER --email [email protected] --no-input | ||
- name: Check main page | ||
run: | | ||
curl_status=$(curl -w '%{http_code}' -o /dev/null -s http://localhost:8000/) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be good to make the port number variable (with 8000
as the default), currently all of them are probably 8000
, but if we introduce any other numbers it's good to have this prepared for that
Fixes maykinmedia/open-api-framework#96