Docker compose - Live reload is not working #657
Comments
|
@asiripanich This is because the setup in the image was super old and obsolete and didn't work any more. Background: I was worried that many of the other docker dev containers put most of their setup logic into the container, not the image, which makes container startup slow. But putting the setup logic into the image will make it hard for different branches to have different dependencies. So I tried an experiment to copy the setup steps into the image in this case. The hope was that when we changed the dependencies in the main code, we would change this as well. Unfortunately, the hope was not realized. I fixed it locally and it works for me. However, the autoreload of the intro screen doesn't work because of I need to put those references to native plugins into the
|
|
Both changes would be nice. Thanks! I can certainly wait. |
|
Sure.
…On Fri, 6 Aug 2021, 16:26 amarin, ***@***.***> wrote:
@WilliamChan4 <https://github.com/WilliamChan4>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#657 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC3O644Q26CULBF4PLIH32TT3N6CTANCNFSM5BU22JLA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>
.
|
shankari
added a commit
to shankari/e-mission-docker
that referenced
this issue
Aug 7, 2021
I originally wanted to put all the dependencies into the image instead of waiting until the container was set up. But different branches could have different dependencies and may not want to upgrade/downgrade. And putting this into the container allows us to use the standard build scripts and not have this bitrot. This file had ENV NODE_VERSION 9.4.0 RUN npm install -g npm@6.0.0 We are currently at export NODE_VERSION=14.7.0 export NPM_VERSION=6.14.8 using the standard scripts, which are regularly tested via CI, allows us to ensure that this dockerfile stays relevant across the many configurations that we currently support. This fixes e-mission/e-mission-docs#657
|
@asiripanich @WillamChan4 Docker changes are Phone changes to allow live-reload on the intro screen are at I have pushed the docker images, but you need to ensure that the phone commit is in your branch, even if you only cherry-pick it. Also, I have only tested this on master, if you are running into issues in your branch, you should probably also ensure that the Please close this issue once you verify that everything works. |
|
@shankari I tried the new changes. The live reload feature only works on 'master' but not my 'rciti1' branch, any idea why? Here the log |
|
Ok I should have read what you mentioned above before making a comment. :) |
|
Can you give me some pointer how to fix this issue with nodejs? I'm seeing this error from running my 'rciti/e-mission-phone@rciti1setup' branch, which has the files in |
|
that is an error in npm. Are you sure you have copied over the setup and re-built your container? The old node version was 9.4.0 and the new one is 14.7.0 |
|
I believe I have all the files from |
|
I believe the setup files are correct, but are you rebuilding the container properly? Because when I run the docker-compose, I get a lot more information before getting to The logs that you have attached seem to be from the old Dockerfile, which actually has a log message saying How did you rebuild and restart your container? |
|
|
|
that is rebuild, how did you restart? |
I also tried to start with the |
|
Ok, I just make sure that |
|
Ok, I can install the phone component now but I'm seeing a whitescreen because of this dep missing error.
|
|
Just FYI, Before doing |
|
That is the translation functionality added by FabMob. I'm not sure exactly what you are doing now. Are you just getting the rciti branch to work with live reload, or are you actually merging changes from master? Not sure whether you had the translate code in your branch before or not... |
|
I have tried both, with only setup files ( |
|
@asiripanich I don't think you can assume that merging all changes from master will Just Work. Isn't that the task that William is working on? wrt only copying setup files, you must have pulled in changes to other files as well, because the setup files should not affect the code or the imports. Ah! I bet you copied Also:
|
|
So I looked at your branch So are you still getting the same error above? Is that the only error? |
|
@asiripanich is it working now? |
|
@shankari I believe that there is something wrong with the |
|
@asiripanich I tested the latest image (3.0.0) with master, and it worked fine. I think you did as well, right? (e-mission/e-mission-docker#18 (comment)) So I don't think that there is anything wrong with the image, just that If you tell me what the error is, I can try and point you in the right direction for the discrepancy, but unfortunately, I can't do much without the current error logs. |
I'm not sure if I was using the latest image. Let me dump the errors for you to see. |
Phone
Server
Error
|
|
You now have another library error which indicates that Can you check the If you are missing any of those directories, it is clearly an issue with bower. Can you use the explicit tag ( |
|
Ah the key is here: It looks like I'm calling this before the activate command I've moved bower after the activate and pushed version 3.0.1 I suspect this worked for me because I had run bower manually to test it out, and the resulting Can you try out v 3.0.1 and share logs again if it doesn't work? I'll restart from scratch tomorrow morning and see what I can find as well. |
|
Thanks! trying now. |
|
Now I have all the libs But I still get a whitescreen on the emulator with these errors: Errors
Docker log
|
|
Ah! Because this |
|
Sure can do. :) |
|
To resolve the |
shankari
added a commit
to shankari/e-mission-phone
that referenced
this issue
Aug 9, 2021
Without this change, we get the following error while running in a docker container ``` Updating bower bower ESUDO Cannot be run with sudo Additional error details: Since bower is a user command, there is no need to execute it with superuser permissions. If you're having permission errors when using bower without sudo, please spend a few minutes learning more about how your system should work and make any necessary repairs. You can however run a command with sudo using "--allow-root" option ``` Since the default user in the container is `root`, this is actually an expected use case for us. I tried to avoid this by calling `bower update --allow-root` in the docker setup script e-mission/e-mission-docker#18 But that ran into multiple inconsistencies ( e-mission/e-mission-docs#657 (comment) e-mission/e-mission-docs#657 (comment) ) In order to avoid more bitrotted containers, just changing the code in the setup script so it works in the container as well This fixes e-mission/e-mission-docs#657
|
Are you going to have to rebuild |
|
not really, the change is mainly in the phone setup, but it would be good to clean up the duplicate call to |
|
Ok, I'm testing this now. |
|
wait, have you merged the change from the phone branch into your fork + branch? |
|
I'm testing the master branch, it didn't work before as well not just my own branches. |
|
I hadn't even merged into master at that time, was waiting for CI to finish running! |
|
Well, that explains this! :p
|
|
ok I can confirm that master works for me with a brand new directory mounted Docker logs
|
|
Testing now.. |
asiripanich
pushed a commit
to rciti/e-mission-phone
that referenced
this issue
Aug 9, 2021
Without this change, we get the following error while running in a docker container ``` Updating bower bower ESUDO Cannot be run with sudo Additional error details: Since bower is a user command, there is no need to execute it with superuser permissions. If you're having permission errors when using bower without sudo, please spend a few minutes learning more about how your system should work and make any necessary repairs. You can however run a command with sudo using "--allow-root" option ``` Since the default user in the container is `root`, this is actually an expected use case for us. I tried to avoid this by calling `bower update --allow-root` in the docker setup script e-mission/e-mission-docker#18 But that ran into multiple inconsistencies ( e-mission/e-mission-docs#657 (comment) e-mission/e-mission-docs#657 (comment) ) In order to avoid more bitrotted containers, just changing the code in the setup script so it works in the container as well This fixes e-mission/e-mission-docs#657
|
It is working now! Thanks for your help @shankari! |
|
No worries, @asiripanich and sorry for the confusion through not testing with a freshly mounted volume... |
Btw, any suggestion how I can fix this? |
|
fix what? Are you still getting an error? |
shankari
added a commit
to shankari/e-mission-docker
that referenced
this issue
Aug 9, 2021
Duplicating the command led to multiple inconsistencies (
e-mission/e-mission-docs#657 (comment)
e-mission/e-mission-docs#657 (comment)
)
So I changed the code in the phone repo instead (e-mission/e-mission-phone#776)
and switched to using only the setup scripts here.
Well, I still get that error about missing the translate module. Anyway, I dont want to waste your time on this as it could be that I didnt do a good job cherry picking all the changes necessary for this to work. |
|
@asiripanich you said
Is it not actually working? After the bower changes, the translate modules should be in Your |
|
Docker logs from running my |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
My docker compose devapp:
The error from Docker logs after a small UI change:
The text was updated successfully, but these errors were encountered: