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

Database seeder output is messed up by new Artisan look #43515

Closed
halloei opened this issue Aug 2, 2022 · 7 comments · Fixed by #43593
Closed

Database seeder output is messed up by new Artisan look #43515

halloei opened this issue Aug 2, 2022 · 7 comments · Fixed by #43593
Assignees
Labels

Comments

@halloei
Copy link
Contributor

halloei commented Aug 2, 2022

  • Laravel Version: 9.22.1
  • PHP Version: 8.1.8
  • Database Driver & Version: MySQL 8.0

Description:

In my database seeders I'm writing output to the console via $this->command->info(). Unfortunately #43065 messed it up:

~$ ./artisan db:seed 

   INFO  Seeding database.  

  Database\Seeders\Users\GroupSeeder ..................................................................................................... 12ms DONE
  Database\Seeders\Users\PermissionSeeder Deleted 0 and created 0 permissions
................................................................................................ 18ms DONE

If you're wondering what I'm doing here: I'm hardcoding the user groups and permissions of my application into seeders. The seeders then keep the database in sync.

Steps To Reproduce:

Create a new seeder an run database seeding:

class TestSeeder extends Seeder
{
    public function run()
    {
        $this->command->info('Lorem ipsum');
    }
}
@ankurk91
Copy link
Contributor

ankurk91 commented Aug 2, 2022

It is not just seeder, we are running artisan command inside migrations via Artisan:call, and if artisan command outputs something, it mess up existing output.

<?php

return new class extends Migration {
   
    public function up()
    {
        Artisan::call(CampaignConfigurationMigrationCommand::class, [
            '--force' => true,
        ], new ConsoleOutput());
    }

};

One way to fix it by using new component intruduce recently.

@ankurk91
Copy link
Contributor

ankurk91 commented Aug 3, 2022

Screenshot from 2022-08-03 10-33-46
Screenshot from 2022-08-03 10-34-08

@nunomaduro
Copy link
Member

@ankurk91 With the console output improvements, we don't really expect "output" in the middle of seeders. Same goes when calling Artisan::call( within seeders, can you consider running that command discarding the output?

@ankurk91
Copy link
Contributor

ankurk91 commented Aug 3, 2022

One can do like

<?php

class DatabaseSeeder extends Seeder
{
   
    public function run()
    {
        // So laravel team does not recommand to do this in seeders?
        $this->command->info('Importing huge data from csv');
  
        Artisan::call(ImportCSVCommand::class, [
            'file' => database_path('raw/huge-csv.csv'),
            '--force' => true,
        ], $this->command->getOutput()); // We should not supply the last argument here? 

  }

I have tried to remove the last parameter from Artisan::call, but now it does not show any output form artisan command.
The artisan command has lines like

$this->line(now().' Starting file import ...');

which we want to append on the terminal when running from seeder. we have been doing this from years already.

we don't really expect "output" in the middle of seeders.

Then we should also deprecate the $this->command in seeder

@halloei
Copy link
Contributor Author

halloei commented Aug 3, 2022

@nunomaduro Doesn't sound like an improvement, at least for me.

Wouldn't it be possible to display the output similar to ./artisan route:list --verbose?

image

The rows that are used to display the middleware (groups) here could be our output instead.

I think this would be a useful addition as there are likely other use cases in the future.

@ankurk91
Copy link
Contributor

ankurk91 commented Aug 4, 2022

ping @nunomaduro
We have not heard from you.

@nunomaduro nunomaduro reopened this Aug 4, 2022
@nunomaduro
Copy link
Member

We are going to address this issue soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants