search

LearnersGuild / idm

identity management service
MIT License
2 stars 24 forks source link

README’s last instruction fails #222

Open jrpool opened 7 years ago

jrpool commented 7 years ago

Instruction 16 (17 if PRs merged), “Add some test users”, crashes. There appear to be multiple possible causes: (1) test/generateUsers line 7 seems to reference './db' when it should reference '../db'. (2) Running the command after correcting './db' produces one of two errors, depending on conditions that I have been unable so far to replicate: (a) In one condition it produces a “ValidationError: Value for [roles][0] must be a string or null”, although debugging shows that roles[0] has a string value on line 52 of test/generateUsers. UnhandledPromiseRejectionWarning is also issued. (b) In another condition it produces an UnhandledPromiseRejectionWarning, “TypeError: (0 , _db.drainPool) is not a function”. In fact, line 7 of test/GenerateUsers.js imports drainPool from ./db (../db?), and tries to invoke it on line 63, but “drainPool”, “drain”, and “pool” (all case-insensitive) appear in the entire project ONLY in these two locations in this file. rethinkDBDash provides for draining the connection pool, but with a drain() method, not a drainPool function (https://github.com/neumino/rethinkdbdash).

jaredatron commented 7 years ago

@prattsj does this ring any bells?

jrpool commented 7 years ago

rethinkDBDash doc says draining pool is normally not necessary. If I comment out the import of drainPool on line 7 and the invocation of drainPool on line 63 of test/generateUsers.js, the script data:users runs without any error report and exits without delay (in condition 2b).

heyheyjp commented 7 years ago

@jrpool: this is an old script that's hardly ever used anymore. I agree it can be improved, but I think the overall data seeding and dev account creation flow warrants a review and perhaps an overhaul. I've got a ticket in the product dev backlog to address it.

jrpool commented 7 years ago

However, “--verbose” is treated as the inviteCode. I am proceeding to debug that problem.

jrpool commented 7 years ago

Sorry, I didn’t see the @prattsj comment. I was working on this for a while per @deadlyicon recommendation. I could stop now, or probably finish current debugging and fixes this afternoon. If the former, I would leave my PR to delete invocation of this script from the onboarding instructions. If the latter, I would restore it. The script does indeed successfully add 15 made-up users per invocation to the DB.

heyheyjp commented 7 years ago

@jrpool: I would recommend pausing as I don't think any fixes you make here will be utilized much in the long term. I'd love to grab some time together in the next few weeks though to design a new data setup flow with you if you're interested.

jrpool commented 7 years ago

@prattsj: That would be terrific. I can interrupt this work, leave notes on findings, and move on. Should I await word from you about a good time to confer?

heyheyjp commented 7 years ago

@jrpool: yes, I'm creating a task in my to-do list to schedule a time to circle back around on this with you.

jrpool commented 7 years ago

Notes for future reference:

test/generateUsers.js:

  1. import {drainPool} from '../db' on line 7 is broken and not needed, so should be deleted.

  2. Same for drainPool() on line 63, and therefore for the “finally” block.

  3. Argument order must be “testing --role=learner --verbose”, not “--verbose --role=learner testing”, in order for new users to have “testing” (instead of “--verbose”) as their invite codes.

  4. Assignment on lines 33–42 leaves role and _ undefined and verbose false when command is “test/generateUsers -- testing --role=learner --verbose”.