Skip to content

Fix development mode in docker for macOs #266

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 30, 2019
Merged

Fix development mode in docker for macOs #266

merged 2 commits into from
Aug 30, 2019

Conversation

smacker
Copy link
Contributor

@smacker smacker commented Aug 22, 2019 •
edited
Loading

I wasn't able to run sourced-ui container in dev mode on my machine.
It took more than 10m to reach npm ci step with consistent close to
100% CPU load.

This commit fixes it by:

  • Mounting source code with consistency: delegated which is the faster
    mode to bind mount
  • Use docker volume for node_modules to avoid syncing it with host
    file system at all

Required changes:

  • Use root user in dev mode always, not only for Linux
  • Set superset user to node_modules volume, by default only root has
    access to it
  • Replace npm ci with npm install because npm ci removes
    node_modules directory before starting installing dependencies. But
    mounted directory can't be removed.

Signed-off-by: Maxim Sukharev max@smacker.ru

  • I have updated the CHANGELOG file according to the conventions in keepachangelog.com
  • This PR contains changes that do not require a mention in the CHANGELOG file

@smacker smacker requested a review from a team August 22, 2019 15:04
@smacker
Copy link
Contributor Author

smacker commented Aug 27, 2019

@se7entyse7en could you please validate it works for you? AFAIK you also use macOs.

@se7entyse7en
Copy link
Contributor

@se7entyse7en could you please validate it works for you? AFAIK you also use macOs.

yup, I'm on OSX. Gonna test it 👍.

@se7entyse7en
Copy link
Contributor

I tried to test this. I built a docker image starting from this branch, and set SUPERSET_ENV: development in the compose file. Then I ran make dev-prepare separately and then restarted using sourced-ce. I tried changing the font-size for the body, html rule in override.less, but nothing happens and the page is not reloaded. By from the console I have this:

Screenshot 2019-08-29 at 12 06 48

What am I missing?

smacker added 2 commits August 29, 2019 12:09
@smacker
Fix development mode in docker for macOs
3f9ab8b 
I wasn't able to run sourced-ui container in dev mode on my machine.
It took more than 10m to reach `npm ci` step with consistent close to
100% CPU load.

This commit fixes it by:
- Mounting source code with `consistency: delegated` which is the faster
mode to bind mount
- Use docker volume for `node_modules` to avoid syncing it with host
file system at all

Required changes:
- Use root user in dev mode always, not only for Linux
- Set superset user to `node_modules` volume, by default only root has
access to it
- Replace `npm ci` with `npm install` because `npm ci` removes
`node_modules` directory before starting installing dependencies. But
mounted directory can't be removed.

Signed-off-by: Maxim Sukharev <max@smacker.ru>
@smacker
Update changelog
3d30e59 
Signed-off-by: Maxim Sukharev <max@smacker.ru>
@smacker
Copy link
Contributor Author

smacker commented Aug 29, 2019

  1. This commit doesn't change how hot reloading works
  2. You shouldn't change anything in compose file
  3. I'm not sure which override.less you are talking about. The one in bblfsh? Then you need to open UAST tab.
  4. I just re-tested with a change in srcd/superset/assets/stylesheets/less/custom-brand.less and

Screenshot 2019-08-29 at 12 38 16

5. I rebased on master & re-pushed so it will use new superset

@se7entyse7en
Copy link
Contributor

se7entyse7en commented Aug 30, 2019 •
edited
Loading

I retested it and worked flawlessly! Thanks! 👏

@smacker
Copy link
Contributor Author

smacker commented Aug 30, 2019

Great. Merging because we have no other macOs guys in the team.

@smacker smacker merged commit a10e00f into src-d:master Aug 30, 2019
@dpordomingo
Copy link
Contributor

@ricardobaeta this fix should also work for you, and speed up your development in Mac.
Could you confirm that hot reloading is now working for you?

btw. Many thanks @smacker for your fix!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@se7entyse7en se7entyse7en se7entyse7en approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@smacker @se7entyse7en @dpordomingo