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

Fix bugs when importing from git repo #41

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mgamlem3
Copy link

TL;DR:

  • Only modify domainlist table if there is content in domainlist.csv
  • Switch DROP TABLE to DELETE FROM
  • Skip first row when importing from csv
  • Add some semicolons

DROP vs DELETE

This addresses issue #40. An explanation can be seen in this comment on the issue. Essentially, dropping the table is more destructive than what is needed when replacing the entries with the data from the csv files saved in git. Using the DELETE command preserves references, triggers, and other important information on the table.

Only modify if there is csv content

I was running into issues where the script would crash if there was no content in domainlist.csv and the script tried to import from it. Obviously, if there is no content in the file it doesn't need to be imported so I just added a check to see if there is anything in the file before attempting an import and modifying the table.

Ignore first line of csv when importing

The first line of the csv file should contain the column names from the SQL table. Skipping this line prevents errors and warnings when importing.

Add some semicolons

I was running into some strange issues with the if statements while making changes and adding these semicolons seemed to solve them.

@stevejenkins
Copy link
Owner

@mgamlem3 Can you take a peek and see what's conflicting with your merge request? I'm reactivating this project after a long pause. Thanks for your great help!

This was causing some strange errors when I was running it. Plus if there is no content to add from git, the table doesn’t need to be modified.
The first row of the csv file should be the column names and should be ignored.
@mgamlem3
Copy link
Author

@stevejenkins it looked like some of the other changes you merged modified the same lines I did. I went ahead and resolved the conflicts while preserving my changes.

@mgamlem3
Copy link
Author

Also, this should close issue #40.

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.

2 participants