Skip to content

joomla: use & require mysql2 instead of mysql#255

Merged
jekyllbot merged 1 commit intojekyll:masterfrom
alejandroerickson:master
Jul 14, 2016
Merged

joomla: use & require mysql2 instead of mysql#255
jekyllbot merged 1 commit intojekyll:masterfrom
alejandroerickson:master

Conversation

@alejandroerickson
Copy link
Contributor

@alejandroerickson alejandroerickson commented May 25, 2016 •
edited
Loading

joomla.rb was using Sequel.mysql, which appears to depend on the gem mysql. This is depracated and replacing mysql with mysql2 works fine; it is already in use for wordpress.rb. I also added the requirement for the mysql2 gem, as this doesn't appear to work without that.

Details of my error message when running the vanilla joomla importer without mysql:

/System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require': LoadError: cannot load such file -- mysql (Sequel::AdapterNotFound)
        from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/adapters/mysql.rb:6:in `rescue in <top (required)>'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/adapters/mysql.rb:3:in `<top (required)>'
        from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require'
        from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/database/connecting.rb:98:in `load_adapter'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/database/connecting.rb:28:in `adapter_class'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/database/connecting.rb:56:in `connect'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/core.rb:108:in `connect'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/core.rb:377:in `adapter_method'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/core.rb:387:in `mysql'
        from /Library/Ruby/Gems/2.0.0/gems/jekyll-import-0.10.0/lib/jekyll-import/importers/joomla.rb:38:in `process'
        from /Library/Ruby/Gems/2.0.0/gems/jekyll-import-0.10.0/lib/jekyll-import/importer.rb:23:in `run'
        from -e:2:in `<main>

`

I suggest, furthermore, that we say MySQL (not the gem) needs to be installed, which is not obvious to people who rent their webserver and database from a third party (i.e., me! heh). I'm not sure how to edit the documentation for joomla.rb, but it definitely needs to go onto individual importer pages, since a user of X CMS will not be looking over the documentation for Y CMS necessarily.

This is my first ever pull request so any comments on how I've done it are welcome.

@parkr
Copy link
Member

parkr commented Jul 14, 2016

LGTM!

@parkr parkr changed the title use mysql2 and require it in joomla joomla: use & require mysql2 instead of mysql Jul 14, 2016
@parkr
Copy link
Member

parkr commented Jul 14, 2016

@jekyllbot: merge

@jekyllbot jekyllbot merged commit 8409bdb into jekyll:master Jul 14, 2016
jekyllbot added a commit that referenced this pull request Jul 14, 2016
@jekyllbot
Update history to reflect merge of #255 [ci skip]
3428bd4
@ienev
Copy link
Contributor

ienev commented May 23, 2017

Looks like the same needs to be done for the Joomla3 converter. I had to recently do some imports and had to manually apply the changes. Is there a specific reason for this not being updated or should I proceed with a PR?

Also I think it's worth adding a port option as there are cases (mine included) where mysql listens on a non-standard port.

@parkr
Copy link
Member

parkr commented May 23, 2017

@ienev I'd be happy to take a look at a PR if you're willing to put one together! Unfortunately I don't have much time to put one together myself.

@ienev ienev mentioned this pull request May 24, 2017
Merged
@jekyll jekyll locked and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

frozen-due-to-age

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alejandroerickson @parkr @ienev @jekyllbot