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 Bug #4569 #4571

Conversation

tgrycuk
Copy link
Contributor

@tgrycuk tgrycuk commented Dec 27, 2023

closes #4569
After some minor changes, building the docker image works.

Screenshots

image

Pull Request Checklist

  1. This code contains UI changes
  2. All visible strings are translated with proper context including data-formatting
  3. Attributes data-test-id are added for new elements
  4. The changes are tested in Chrome/Firefox/Safari browsers and in light/dark mode
  5. Your code works with the latest stable version of the core
  6. I added changesets file (instructions in contribution guide

Test environment config

API_URI=https://automation-dashboard.staging.saleor.cloud/graphql/
APPS_MARKETPLACE_API_URI=https://apps.staging.saleor.io/api/v2/saleor-apps

Do you want to run more stable tests?

To run all tests, just select the stable checkbox. To speed up tests, increase the number of containers. Tests will be re-run only when the "run e2e" label is added.

  1. stable
  2. app
  3. attribute
  4. category
  5. collection
  6. customer
  7. giftCard
  8. homePage
  9. login
  10. menuNavigation
  11. navigation
  12. orders
  13. pages
  14. payments
  15. permissions
  16. plugins
  17. productType
  18. products
  19. sales
  20. shipping
  21. translations
  22. variants
  23. vouchers

CONTAINERS=2

@tgrycuk tgrycuk requested a review from a team as a code owner December 27, 2023 07:21
Copy link

changeset-bot bot commented Dec 27, 2023

🦋 Changeset detected

Latest commit: c335eee

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
saleor-dashboard Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@CLAassistant
Copy link

CLAassistant commented Dec 27, 2023

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@@ -1,7 +1,7 @@
FROM node:18-alpine as builder
RUN apk --no-cache add bash
Copy link
Member

Choose a reason for hiding this comment

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

why do you need to install bash?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Without bash this script cannot execute. And this script is executed while building the image.

image

Copy link
Member

Choose a reason for hiding this comment

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

basically, we don't use sentry at all within the docker but the script is being executed even if it skips the sentry-related scripts. Ok let's keep the bash for now, will perhaps wok on some workaround later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My goal was to make a docker image build. I didn't want to interfere with the script. I left the script changes for other issues.

@andrzejewsky
Copy link
Member

Hey @tgrycuk thanks for contribution! Please accept CLA + add the changeset file (as patch)

@andrzejewsky andrzejewsky self-assigned this Dec 27, 2023
@andrzejewsky andrzejewsky self-requested a review December 27, 2023 11:37
@andrzejewsky andrzejewsky merged commit 8e0e208 into saleor:main Dec 27, 2023
20 of 21 checks passed
andrzejewsky added a commit that referenced this pull request Jan 12, 2024
* Fix Bug #4569

* add changeset

* Update unable-to-build-docker-image .md

---------

Co-authored-by: Tomasz Grycuk <[email protected]>
Co-authored-by: Patryk Andrzejewski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 👀 In Review
Development

Successfully merging this pull request may close these issues.

[Bug]: Unable to Build Docker Image
4 participants