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

Add devcontainer.json for GitHub/VSCode codespaces #251

Merged
merged 11 commits into from Jan 2, 2023

Conversation

andrewm4894
Copy link
Collaborator

Related to #242

This PR adds a minimal devcontainer.json to enable running the frontend-dev via a GitHub codespace.

There is probably a bit more work to do like in here: https://stackoverflow.com/a/68864132/1919374 to extend this to a more complete or full dev env where any service could be worked on.

@yk
Copy link
Collaborator

yk commented Jan 2, 2023

Thank you, looks really neat already!

  • could you run pre-commit run --all-files to make linters happy?
  • who do you intend this to use?
    • for people wanting to develop on the backend, the backend-dev service needs to be started, along with the run-local.sh in the backend development folder
    • for people wanting to develop on the website, the frontend-dev service needs to be started, along with npx prisma db push and npm run dev in the website folder
    • for people wanting an end-to-end demo, the entire docker-compose file needs to be started

so we need to decide a bit who this is for and then add/change the things so that a smooth setup is possible. Or is the idea that e.g. the user starts something like npm run dev themselves once they're in the dev container?

@andrewm4894
Copy link
Collaborator Author

andrewm4894 commented Jan 2, 2023

Main benefits (i think) this gives:

  • Truely standardised environment in which to test, develop or even just try out and play with it in some way.
  • No need to download and take up space on your laptop (avoid having lots of docker images locally eating space and internet bandwidth pulling them all down).
  • Ability to just easily hope into a throw away dev env on a feature branch - to test, dev or give feedback etc.
  • A really simple happy path that users looking to help could be guided on - e.g a few screenshots and they should be looking at what we might expect, less moving parts than their own laptop.
  • If you are working on multiple feature branches it's nice to just have separate codespaces for each so you can easily jump between them. Can be annoying or add friction when you try do this locally on your laptop.
  • they are cool and new hot thing :)

It could of course be that maybe this is getting a bit too fancy too soon too perhaps which could be a fine objection if ends up being something that needs more maintenance overhead without a clear and proven benefit.

Am going to see if i can get it so that i can run that pre-commit hook from within the codespace (is a good example of a dev tool we'd need to add to the default codespace image etc.).

@andrewm4894 andrewm4894 marked this pull request as draft January 2, 2023 15:10
@andrewm4894
Copy link
Collaborator Author

Converting PR to draft to reduce any noise while there can be discussion in here if it's something that is needed or wanted. If so then I'm happy to try finalize and updraft once we happy it make sense.

@andrewm4894
Copy link
Collaborator Author

andrewm4894 commented Jan 2, 2023

Or is the idea that e.g. the user starts something like npm run dev themselves once they're in the dev container

This for sure could be one option - once you have your codespace then you can run whatever commands you might to spin or some dev service just like you might on your laptop.

I think the fact the frontend-dev seems to depend on most of the other images I think means that as it is it will just launch you into frontend-dev where most other services have already been built.

I think what's suggested in here could be good but could be a bit more complex.

For now i think it could make sense to think of this as an easy way to just get yourself either an ephemeral (and in cloud as opposed to laptop) backend-dev or frontend-dev in which you could test or do some dev. Am going to see if is a way to parameterize the service such that you could just ask GH codespaces to give you either a backend-dev or frontend-dev e.g since these dev envs already in the docker compose the codespaces stuff should just try leverage them so can be as minimal overhead here and using a codespace just becomes another easy option that could be handy and is not that much hassle to maintain and not going to break anything anyway.

Copy link
Collaborator

@yk yk left a comment

Choose a reason for hiding this comment

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

SGTM, thank you!

@andrewm4894 andrewm4894 marked this pull request as ready for review January 2, 2023 16:00
@andrewm4894
Copy link
Collaborator Author

trying to run pre-commit but hit this: #280

will see if maybe is some node version or stuff i can figure out on my end.

@yk
Copy link
Collaborator

yk commented Jan 2, 2023

pre-commit on github seems happy. good to merge?

@andrewm4894
Copy link
Collaborator Author

Yep happy to merge - unsure exactly how pre-commit seems happy now, assuming its a GH action or something (?).

Anyway should be good to merge.

@andrewm4894 andrewm4894 merged commit 571dcc2 into LAION-AI:main Jan 2, 2023
@andrewm4894 andrewm4894 deleted the add-devcontainer branch January 2, 2023 18:01
@eaglespy21
Copy link

eaglespy21 commented Jan 3, 2023

Hi, @andrewm4894
I was looking forward to this change, but with this change the code space that is created seems to be broken.
Before this change I was able to create a code space using the "default" template, build the container and open the website up.
But now the workspace cannot be built, because it's missing a bunch of dependencies like git and docker.
So, I am unable to build the code.

This is the first time I am using code spaces. Docker build & run worked seamlessly OOB before this change. Either this change broke something, or I am using code spaces incorrectly. If it's the latter, please can you update the QuickStart documentation on how to use this change and code spaces?

Thank you!

tl;dr before this change default code space allowed me to follow the readme, build and run, with this change I am not able to do that anymore because of missing dependencies (starting with git, docker, etc.)

@andrewm4894
Copy link
Collaborator Author

andrewm4894 commented Jan 3, 2023

hey @eaglespy21 i think this PR (#333) might fix and superseed your issues above.

i have just tested it and all looks good - let me know if you hit any issues once its merged or you can try make a code space on my feature branch here if you dont want to wait until its merged: https://github.com/andrewm4894/Open-Assistant/tree/devcontainer-improvements

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.

None yet

3 participants