michabbb / laravel-scheduler-watcher

Easy logging & monitoring of your Laravel cronjobs
MIT License
8 stars 4 forks source link

table names conflict #1

Closed jamesj2 closed 4 years ago

jamesj2 commented 4 years ago

The migrations use generic table names like 'jobs'. I've already have a table named jobs in my database. Could it be changed to use a prefix?

michabbb commented 4 years ago

hi James,

that's a very good idea, thank you 👍 this is my very first laravel package and there a many things I still have to learn.

so I am happy to learn from your request. do you know if it's possible to use dynamic table names in migration files? 🤔 maybe there is a way so everybody can choose to use a prefix or not and set the name of this prefix.

but maybe it makes sense to you, if I explain my intention here, why I didn't thought about a prefix:

I have several stand alone laravel apps, if each app has its own database, it wouldn't make sense monitoring each app in its own database. in case my package has major changes, you have to change several databases, so much duplicated work.

if you use a independent database to monitor all your laravel cron jobs, you only have one database to backup, to monitor, to change, whatever... only one database, and not "many" 😏

so in that separate database, prefixes don't play a role, because it's exclusively made for a few monitor tables. and nothing else, so there you will never have any conflicts.

I will check about a prefix, but my recommendation anyway is, to use separate database, because when you start monitoring more than one app, it's always the better choice.

cheers, Micha

jamesj2 commented 4 years ago

I'm fairly new to Laravel myself and have been using PHP for a long time. I do understand using different databases can be useful when using multiple projects but you also have to manage access to it. Which gets more complicated when setting up multiple environments. Also some online hosting sites only allow a specific number of databases.

My suggestion would be to add the tables names in the config file and reference it where ever it's used.

Something like...

class CreateJobsTable extends Migration {

    /**
     * Run the migrations.
     *
     * @return void
     */
    public function up()
    {
        Schema::connection(config('scheduler-watcher.database'))->create(config('scheduler-watcher.jobs.table_name'), function(Blueprint $table)
        {
            $table->integer('job_id', true);
            $table->char('job_md5', 32)->unique('UK_jobs_job_md5');
            $table->string('job_name');
            $table->string('job_command')->nullable();
            $table->dateTime('job_db_created')->nullable();
        });
    }

    /**
     * Reverse the migrations.
     *
     * @return void
     */
    public function down()
    {
        Schema::connection(config('scheduler-watcher.database'))->drop(config('scheduler-watcher.jobs.table_name'));
    }
}

You would also need to set the table name dynamically in the model constructor.

class jobs extends Model {
    protected $connection;
    protected $table;

    public function __construct()
    {
        $this->connection = config('scheduler-watcher.database');
        $this->table = config('scheduler-watcher.jobs.table_name');
        parent::__construct();
    }
}
michabbb commented 4 years ago

hi @jamesj2 , thanks for your input, i will check if that works... but i guess, i won´t be able to look into it till next weekend....

michabbb commented 4 years ago

hey @jamesj2, added the table prefix, I didn´t have the time (for now) to test everything. so I would be happy if you could check my last commit and test if everything works as expected. thanks!

jamesj2 commented 4 years ago

I was able to test the migrations after fixing a class naming conflict and they are working.

michabbb commented 4 years ago

@jamesj2 thanks for testing! i changed the migration files, so i guess, we can close this issue now ? 😏