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

Docker config refactor #104

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

kinghat
Copy link
Contributor

@kinghat kinghat commented Apr 14, 2022

more cleanup of the docker situation. afaict the only thing left is the API_URL at yarn build to deal with for publishing the images.

@MaxLeiter
Copy link
Owner

@reeseovine sorry for the ping again... would you mind checking this is what we expect when you have the chance? I want to be sure we're not shipping unnecessarily large docker images or anything.

@reeseovine
Copy link
Contributor

reeseovine commented Apr 14, 2022

That's odd, I wonder why the images are so much bigger. I think it might have to do with unnecessary files being carried over between stages, like package cache maybe.

Edit: Ok yeah I think it's leaving in the npm dev dependencies after they're needed.

@kinghat
Copy link
Contributor Author

kinghat commented Apr 14, 2022

That's odd, I wonder why the images are so much bigger. I think it might have to do with unnecessary files being carried over between stages, like package cache maybe.

oh dang, you measured the differences between master and this pr? i looked and the final images were exactly the same size.

@reeseovine
Copy link
Contributor

oh dang, you measured the differences between master and this pr? i looked and the final images were exactly the same size.

Yeah, here's main:
image

and here's your branch:
image

I think that's how you do it? I'm no docker expert.

@kinghat
Copy link
Contributor Author

kinghat commented Apr 14, 2022

oh dang, you measured the differences between master and this pr? i looked and the final images were exactly the same size.

Yeah, here's main: image

and here's your branch: image

I think that's how you do it? I'm no docker expert.

ya you are correct. im not sure what i overlooked when comparing earlier. thanks for the catch @reeseovine 🙏 ill fix it this evening.

@reeseovine
Copy link
Contributor

reeseovine commented Apr 14, 2022

Ah here we go, I found the culprit
944.7M /usr/local/share/.cache/yarn, 944.7M /usr/local/share/.cache

Just rm that near the end of the dockerfile and it should be good!

This is in the client btw. The server might be the same but it's worth looking.

@MaxLeiter
Copy link
Owner

@kinghat yarn cache clean should do the job

@kinghat
Copy link
Contributor Author

kinghat commented Apr 15, 2022

Ah here we go, I found the culprit 944.7M /usr/local/share/.cache/yarn, 944.7M /usr/local/share/.cache

Just rm that near the end of the dockerfile and it should be good!

This is in the client btw. The server might be the same but it's worth looking.

@kinghat yarn cache clean should do the job

i fiddled with that and couldnt get it to reduce the size. it is better than the last stage, the image to be published, is separate from the deps and build stages anyways. sorry again 🤦‍♂️

@reeseovine
Copy link
Contributor

reeseovine commented Apr 15, 2022

couldnt get it to reduce the size

Were you still looking at the size of the old one? I just built the latest changes and it looks much better:
image

Server looks good too:
image

@kinghat
Copy link
Contributor Author

kinghat commented Apr 15, 2022

couldnt get it to reduce the size

Were you still looking at the size of the old one? I just built the latest changes and it looks much better: image

Server looks good too: image

thanks for building again. i dont think so as i was paying attention to the created time. but like the original issue, i thought they were the same size before i sent the PR, so anythings possible 😅

@reeseovine
Copy link
Contributor

Oh do you mean it's the same size as before you started this PR? Personally I think they're both fine as they are in terms of size, which should answer Max's original question for me 😁

@kinghat
Copy link
Contributor Author

kinghat commented Apr 15, 2022

Oh do you mean it's the same size as before you started this PR? Personally I think they're both fine as they are in terms of size, which should answer Max's original question for me grin

ya its the same size as before i pushed, but also, i double checked the size before i pushed and i must have looked at the wrong thing because they were not, as you found out.

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.

3 participants