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

Add database connection #568

Merged
merged 6 commits into from
Jul 23, 2019
Merged

Add database connection #568

merged 6 commits into from
Jul 23, 2019

Conversation

leMaur
Copy link
Contributor

@leMaur leMaur commented Jul 22, 2019

In case of project with multi database connection, we need the ability to specify the database connection.

Copy link
Collaborator

@Gummibeer Gummibeer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work. :)

Just to link everything: #549 & #95

.gitignore Outdated Show resolved Hide resolved
config/activitylog.php Outdated Show resolved Hide resolved
@leMaur
Copy link
Contributor Author

leMaur commented Jul 22, 2019

Thanks for your work. :)

Just to link everything: #549 & #95

Thank @Gummibeer and Spatie to mantain this usefull package.

I'm glad to contribute! 😄

@Gummibeer Gummibeer merged commit 5dfff99 into spatie:master Jul 23, 2019
@Gummibeer
Copy link
Collaborator

* This is the database connection that will be used by the migration and
* the Activity model shipped with this package.
*/
'database_connection' => env('ACTIVITY_LOGGER_DB_CONNECTION', 'mysql'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should default to DB_CONNECTION instead of mysql, since a lot of unit tests set only DB_CONNECTION in phpunit.xml and therefore test suites will break without explicitly setting this config. Case in point, sqlite :memory: connections.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - now the philosophic question: should we use another env() in the fallback or just use DB_CONNECTION?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @drbyte to pointed out. I was in doubt when I did the pull request but I think this is best solution!
I will think twice about this config. It'll be helpfull for future projects! 👍

@lk77
Copy link

lk77 commented Oct 11, 2019

Hello,

since this change we have an issue on v3.7.1:

Database [mysql] not configured.

this is a breaking change and should have been pushed to v4.0 not to v3.7.1

@Gummibeer
Copy link
Collaborator

#616 is already open to fix it. Will release it today or tomorrow. I'm sorry for this inconvenience. Until then you can fix an earlier version or add ACTIVITY_LOGGER_DB_CONNECTION to your .env.

@bakhtiyor
Copy link

I am using a separate database for storing information about activity logs using ACTIVITY_LOGGER_DB_CONNECTION. But once I try to retrieve information about activitylog->causer->name or activitylog->subject->name I am getting an error saying that the base table or view not found in the database I specified in ACTIVITY_LOGGER_DB_CONNECTION which is true, because they are in my "main" database. How to fix this error?

@Gummibeer
Copy link
Collaborator

Gummibeer commented Nov 2, 2020

I hope that Laravel respects the given connections on a relationship. 🤔🧐

Could you check if you can create a test ase to reproduce it?
Some temporary ideas: define the connection on the related models.

But at all please open a new issue. 😉

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

Successfully merging this pull request may close these issues.

5 participants