Skip to content

Conversation

@rusrushal13
Copy link
Contributor

Issue: BB-2320

Testing Instructions:

  • Try running problem builder as given in the README, check everything is running fine.

Reviewers:

@rusrushal13 rusrushal13 requested a review from xitij2000 May 8, 2020 08:23
Copy link
Member

@xitij2000 xitij2000 left a comment

Choose a reason for hiding this comment

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

I think the readme doesn't work as of yet. I faced a number of failures:

  1. The xblock-sdk's make install command files. The incorrect version is used here, it should be 0.1.8 instead of 0.1.7
  2. The migration command failed since the current readme asks you to override DATABASES['default']['NAME'] = 'var/workbench.db'

@rusrushal13 rusrushal13 force-pushed the rushal/BB-2230-MySQL-README branch from dfda39a to 724801e Compare May 19, 2020 21:53
@rusrushal13
Copy link
Contributor Author

I think the readme doesn't work as of yet. I faced a number of failures:

1. The xblock-sdk's `make install` command files. The incorrect version is used here, it should be 0.1.8 instead of 0.1.7

2. The migration command failed since the current readme asks you to override `DATABASES['default']['NAME'] = 'var/workbench.db'`

@xitij2000 Thanks for the review, I had some issues, which @lgp171188 helped me to resolve it; Now I have tested it on an Ubuntu 16.04(Xenial64) VM with python 3.5.2 installed and it works there fine. Let me know if you face any issues :)

README.md Outdated
Comment on lines 102 to 108
By default, the `xblock-sdk` uses the SQLite database but MySQL
can be used by specifying an environment variable `WORKBENCH_DATABASES` in
the following format.

```bash
export WORKBENCH_DATABASES='{"default": {"ENGINE": "django.db.backends.mysql", "NAME": "db", "USER": "root", "PASSWORD": "rootpw", "HOST": "127.0.0.1", "PORT": "3306"}}'
```
Copy link
Contributor

Choose a reason for hiding this comment

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

@rusrushal13, I think this paragraph, can be moved to after the docker run command. What do you say?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First I thought the same, but running the export command before running the database looks better to me, that's why I have kept it before the docker command. What do you think?

Copy link
Member

@xitij2000 xitij2000 left a comment

Choose a reason for hiding this comment

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

👍 The changes look good. Could you also update xblock-sdk submodule here to the same version as that in the requirements file, and fix the port in the readme? Other than that it is good to go.

  • I tested this: tested setting up based on readme
  • I read through the code
  • Includes documentation

@rusrushal13 rusrushal13 merged commit e3fb0c4 into master May 25, 2020
musmanmalik added a commit to musmanmalik/problem-builder that referenced this pull request Jun 1, 2020
[BB-2320] Added the MySQL Readme for Problem builder (open-craft#282)
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.

4 participants