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

Does not show error message when database throws an error #34

Open
glupeksha opened this issue Sep 9, 2020 · 8 comments
Open

Does not show error message when database throws an error #34

glupeksha opened this issue Sep 9, 2020 · 8 comments

Comments

@glupeksha
Copy link

I got the "Seeded:" message but the csv file was not seeded because of a database error(forgotten to make a column nullable). I wasted lot of time to figure out the error. It would be wonderful it it can throw if there is any database error.

PS: Thanks for providing this library for free. It was really helpful.

@dashart-ke
Copy link

Same issue, I struggled above today.
But anyway, thanks for the great library!

@a21ns1g4ts
Copy link

Shot in the dark

@ben182
Copy link

ben182 commented Nov 19, 2020

Same for me. Spend an hour to find the bug that caused my tests to fail after a laravel 8 upgrade. The seeds were not working anymore because the seed folder got renamed to "seeders". Please throw an error if the importer can not find the csv file. This is quite easy to implement. At the moment this is only logged

Log::error("CSV insert failed: CSV " . $filename . " does not exist or is not readable.");

Please throw an error here

if ($handle === false) {
return false;
}

@Flynsarmy
Copy link
Owner

@ben182 The problem described in your comment is fixed through e1774b1

Your comment should have been its own issue as it's different to OPs though.

@Flynsarmy
Copy link
Owner

I don't really like the idea of throwing an Exception on DB error as this may result in the CSV being partially imported which will complicate matters further when a second attempt to import is made.

Do we have any other suggestions?

@dashart-ke
Copy link

Maybe an echo output of a warning with some error details could be help nor write of same details to the laravel log files + a note to the cmd line.
The current silent fail is the main problem.

@moetanahy
Copy link

Hi,

I'm using version 2.0.5 and it's the first time I try out the package. It's not throwing any error nor importing the data into the database.

I just get a "Seeded:" notice.

I'm using PHP8 with Laravel 8.27.

Has this been fixed in a non-released version?

Thanks in advance.

@jspaans
Copy link

jspaans commented Aug 18, 2021

I don't really like the idea of throwing an Exception on DB error as this may result in the CSV being partially imported which will complicate matters further when a second attempt to import is made.

Do we have any other suggestions?

Make it configurable with a flag to either raise the exception or silently ignore it? Also, seedFromCsv returns $success, but also the run function should return this.

I'm using the CsvSeeder to load test data for development, if the data is invalid (failing database constraints) I would like the seeding process to halt so the issue can be resolved.

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

No branches or pull requests

7 participants