instructure / canvas-data-loader

An Example Application to download data from Canvas Data, and import it into a Database.
MIT License
17 stars 18 forks source link

Missing column error #9

Closed stuartf closed 6 years ago

stuartf commented 6 years ago

As of yesterday I get cdl_runner::db_client: Error(Db(DbError { severity: "ERROR", parsed_severity: Some(Error), code: SqlState("42703"), message: "column \"external_tool_id\" of relation \"assignment_dim\" does not exist", detail: None, hint: None, position: Some(Normal(182)), where_: None, schema: None, table: None, column: None, datatype: None, constraint: None, file: Some("parse_target.c"), line: Some(954), routine: Some("checkInsertTargets") })) when running the loader.

Mythra commented 6 years ago

Hey @stuartf ,

This is actually a purposeful footmine that we should probably get rid of, since more people adopted the tool than expected as an interim solution. It was originally built to not update the schema automatically (so users would be aware of schema changes, and give them a motivation for changing to a fully managed tool).

Essentially what's happening here is the tool doesn't drop the old version of the "non-volatile" tables when a new schema is published (but reads the files with the new schema). Thus leading it to insert a column that doesn't actually exist in the schema. (The "quick fix" here is to drop all the tables that added/dropped rows mentioned in the release notes: https://community.canvaslms.com/docs/DOC-14270-canvas-data-release-notes-2018-02-27 ).

I'll talk internally to our product manager over data, and see if she would like for us to fix the footgun, or just note it in the readme (for reasons mentioned above). Because users should at least be aware of this limitation if we decide to not merge a fix in upstream.

jlongland commented 6 years ago

I'd support the change to update the schema automatically.

Mythra commented 6 years ago

After some internal discussion we've decided that upgrading the db in place is fine, and we don't think it'll get people too attached.

I'll be pushing something up for this soon.

Mythra commented 6 years ago

This has been fixed in the latest commit.