Skip to content
This repository has been archived by the owner on Sep 1, 2022. It is now read-only.

Check differences between source data and the database #98

Merged
merged 2 commits into from
Sep 30, 2020

Conversation

zkagin
Copy link
Collaborator

@zkagin zkagin commented Aug 18, 2020

Depends on #96
Fixes #85

Adds "diffing" logic to check the differences between source data and the database, and returns data that should either be deleted/archived or created.

Tests pass and should cover the corner cases I can think of so far.

A few notes for future cleanup:

  • Right now the CourseAliases object gets created from within the Base, which is not a great pattern. Eventually I'd like to refactor this so that a Controller object creates all of the endpoints and can then pass in the Aliases information that is needed.
  • The clearing of databases in the test suite should happen in a teardown() function rather than in the test itself, but I haven't yet found a good way to clear the whole database. Until I do that, just dropping the table from the associated endpoint seems to work well enough.

@zkagin zkagin requested a review from dchess August 18, 2020 03:13
@zkagin zkagin force-pushed the identify_changes_to_be_made branch 2 times, most recently from b4a05ef to 8e44a79 Compare August 26, 2020 19:05
@zkagin zkagin changed the base branch from add_sync_option to master August 26, 2020 23:20
@zkagin zkagin force-pushed the identify_changes_to_be_made branch 2 times, most recently from d7a16ea to 6de3816 Compare August 29, 2020 01:03
@dchess
Copy link
Collaborator

dchess commented Sep 24, 2020

@zkagin Had a chance to test this out this morning and the results look good. Although we wouldn't want to use the to_delete portion for courses (only students).

However, when I try running the test suite locally I am getting a failed test for the to_create assertion (line 170 of test_all.py).

@dchess dchess changed the base branch from master to main September 25, 2020 03:18
@dchess
Copy link
Collaborator

dchess commented Sep 25, 2020

@zkagin Switching this to merge to main now that it is the default branch instead of master.

@zkagin
Copy link
Collaborator Author

zkagin commented Sep 26, 2020

Good catch, I changed something in the code and it did break the tests (the columns are different). Concerningly, while the check correctly runs the test and the test correctly fails, the check itself passes even though it has this:

app_1       | ========================= 1 failed, 15 passed in 8.00s =========================
google_classroom_app_1 exited with code 1
Stopping google_classroom_database_1 ... 

Stopping google_classroom_database_1 ... done
Aborting on container exit...
Removing google_classroom_app_1      ... 
Removing google_classroom_database_1 ... 

Removing google_classroom_app_1      ... done

Removing google_classroom_database_1 ... done
Removing network google_classroom_default

The exit code 1 should cause the check to fail. I'll try and dig into it, but for now I'll make sure to manually run tests before I submit.

@zkagin
Copy link
Collaborator Author

zkagin commented Sep 30, 2020

@dchess For the time being I will run tests manually to make sure they pass until I can figure out what's going on with the Github Action.

This should be ready for your review again.

@dchess dchess merged commit de7033b into main Sep 30, 2020
@dchess dchess deleted the identify_changes_to_be_made branch September 30, 2020 14:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating Diffing Logic to Check Difference Between Stored List of Items and Provided List
2 participants