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

feat: introduce 'tutor dev quickstart' (also, some extra docs) #627

Merged
merged 2 commits into from
Apr 19, 2022

Conversation

kdmccormick
Copy link
Collaborator

Description

Fixes openedx-unsupported/wg-developer-experience#58

There are three commits. The first two just add some supplementary doc updates, which I could remove from this changeset if they don't seem like a good idea. The third commit introduces tutor dev quickstart along with the most critical documentation updates.

Implementation Notes

I wanted to tailor tutor dev quickstart to the development use case, so there are some differences between it and tutor local quickstart:

  1. it starts dev containers instead of local ones (duh),
  2. it builds the openedx-dev image, and
  3. it assumes No for the the "Is this a production platform?" question,
  4. it offers to import the demo course and create a supseruser.

Difference 1 is the only thing that really needed to be changed, so if there are objections to 2-4 do let me know.

In order to implement differences 3-4 I needed some way of passing to tutor config save that fact that we're in a developer context. Instead of cluttering tutor config save with a new command-line option, I opted to expose an is_dev() method on the Context class, which is used to tell whether developer-mode assumptions should be made. Let me know if this seems like a bad idea.

@kdmccormick
Copy link
Collaborator Author

@regisb This PR's also ready for review.

README.rst Outdated
@@ -63,10 +63,21 @@ Features
Quickstart
----------

For Open edX deployers
~~~~~~~~~~~~~~~~~~~~~~

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a big fan of making the quickstart instructions more complex. They are purposely very short; that's because there are many people who don't care about running Tutor in dev mode. So I suggest getting rid of this commit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That makes sense, I'll remove this.

Still, I think the readme should have some link to point Open edX developers in the right direction. Currently, the Open edX development page is 3+ clicks away. How would you feel about linking right to that page, either in the first sentence ("...both for production and local development.") or via a new bullet point in the Features list?

docs/dev.rst Outdated Show resolved Hide resolved
docs/dev.rst Outdated Show resolved Hide resolved
docs/dev.rst Outdated Show resolved Hide resolved
docs/dev.rst Outdated Show resolved Hide resolved
docs/dev.rst Outdated Show resolved Hide resolved
docs/dev.rst Outdated Show resolved Hide resolved
docs/intro.rst Show resolved Hide resolved
tutor/commands/config.py Outdated Show resolved Hide resolved
docs/dev.rst Outdated Show resolved Hide resolved
@bradenmacdonald
Copy link
Contributor

@kdmccormick @regisb What do you think about tutor dev quickstart offering an interactive option to use (or even create) a copy of edx-platform on the user's host computer? e.g. the old devstack used to clone edx-platform for you, and I think that for Open edX developers, having an editable version of the platform is important. (I would actually guess that most people who want to use tutor nightly are also wanting to have an editable/dev version of edx-platform).

However, currently the instructions for doing so are complicated (and not referenced on the Nightly page). So I was thinking something like a prompt which says: "Do you have a local checkout of the edx-platform source (master branch) that you want to use with Tutor Nightly? If so, enter the full path to it [e.g. /home/regis/dev/edx-platform] or press enter to skip:"

And then if you enter a path, it can create the docker-compose.override.yml so that everything "just works".

@kdmccormick
Copy link
Collaborator Author

kdmccormick commented Apr 18, 2022

@bradenmacdonald That's a really interesting idea. I agree with your point that mounting code locally is currently too complicated. We have work in flight to address it, with a proposal syntax like:

$ tutor dev start -d -m /path/to/my/edx-platform -m /path/to/my/other-ida-repo

So, I am torn on your suggestion. On one hand, it would allow developers to "set and forget" repository mounts, which would certainly lower the start-up effort for new users. On the other hand, running services from built-in container code by-default is one of the reasons Tutor is less resource-intensive and (IMO) more reliable than Devstack (which host-mounts code for every single service), so encouraging developers to explicitly mount in host directories as-needed might be better. @regisb , what do you think?

Add `tutor dev quickstart` command, which is equivalent to
`tutor local quickstart`, but uses dev containers instead
of local production ones and includes some other small
differences for the convience of Open edX developers.
This should remove some friction
from the Open edX development setup process, which previously
required that users provision using local producation
containers but then stop them and switch to dev containers:
 * tutor local quickstart
 * tutor local stop
 * tutor dev start -d

Document the command and its improved workflow in
./docs/tutorials/nightly.rst

Fixes openedx-unsupported/wg-developer-experience#58
@bradenmacdonald
Copy link
Contributor

bradenmacdonald commented Apr 19, 2022

@kdmccormick I see, that does make a lot of sense. Sorry I missed the existing discussion; we can follow up in openedx-unsupported/wg-developer-experience#43.

Copy link
Contributor

@regisb regisb left a comment

Choose a reason for hiding this comment

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

Can I just say that I love all the changes that we are making?

tutor/commands/dev.py Show resolved Hide resolved
@regisb regisb merged commit df0e26c into overhangio:master Apr 19, 2022
@kdmccormick kdmccormick deleted the kdmccormick/dev-quickstart branch April 19, 2022 14:59
regisb added a commit that referenced this pull request Dec 13, 2022
I have no idea why we decided to kickstart a separate build. The image will be
built anyway at the next step because we run `docker compose up --build` in
`tutor dev start`.

The build step was introduced in this PR: #627
regisb added a commit that referenced this pull request Dec 13, 2022
I have no idea why we decided to kickstart a separate build. The image will be
built anyway at the next step because we run `docker compose up --build` in
`tutor dev start`.

The build step was introduced in this PR: #627
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.

Add 'tutor dev quickstart' command
3 participants