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 after_initialize hooks #168

Closed
wants to merge 3 commits into from

Conversation

Yuki-Inoue
Copy link
Contributor

@Yuki-Inoue Yuki-Inoue commented Jan 29, 2019

Hello iruby maintainers,

Recently, I've been writing a gem to integrate rails into jupyter (or jupyter to rails). https://github.com/Yuki-Inoue/jupyter_on_rails

Basically, this gem makes it easy to use jupyter on a rails app, by defining a rake task to start a jupyter server configured for that rails app, so that a kernel for loading the rails app (something like rails c)will be available. (I think this would solve #52 problem.)

There, I noticed some gems require the IRuby::Kernel to be started prior to those gem's initialization, which makes it impossible for the Rails app to be initialized by the boot_file of the kernel.

require boot_file if boot_file

I worked this around by extending the IRuby::Kernel#run method, and doing all the Rails initialization just before the kernel starts the dispatch loop, but I think this implementation is not so clean. (FWIW, this is the code for such hack: https://github.com/Yuki-Inoue/jupyter_on_rails/blob/master/lib/jupyter_on_rails/boot.rb and https://github.com/Yuki-Inoue/jupyter_on_rails/blob/master/lib/jupyter_on_rails/iruby_kernel_extention.rb )

IMHO, there will be also some other cases where someone wants to add some hooks to the kernel startup process, on after the Kernel creation but before the actual Kernel#run, I thought it would be beneficial to implement a hook of such in this kernel itself.

So, I've created this PR, which basically allows the boot of kernel to add hook against after_initialize. FWIW, with this feature, my implementation would not need to hack the Kernel class, so it would be much more clean, as seen in this PR: https://github.com/Yuki-Inoue/jupyter_on_rails/pull/2/files

... Any opinion?

@mrkn mrkn self-requested a review January 30, 2019 15:02
@mrkn
Copy link
Contributor

mrkn commented Jan 31, 2019

@Yuki-Inoue Could you write the unit test for this new feature?

Although the CI doesn't work well, I want to collect tests for new features.

@kojix2
Copy link
Member

kojix2 commented Sep 15, 2019

I think the Jupyter-Rails integration is important for many Rubyists. Though I'm a hobbyist programmer and I don't know Rails. So I'm afraid I can't help you much.
@Yuki-Inoue
You look busy these days. I mean, I saw your GitHub calendar. Your coding time seems to be decreasing these months. Could you write a test code?

@horacio
Copy link

horacio commented Apr 26, 2020

@Yuki-Inoue @kojix2 I can help adding the corresponding tests if needed.

@mrkn
Copy link
Contributor

mrkn commented Apr 27, 2020

@horacio It's very helpful for us.

@Yuki-Inoue
Copy link
Contributor Author

Hi, sorry for late reply... As kojix2 mentioned, I had been a bit busy and couldn't spare time...

Writing test for this feature is not trivial ( at least for me ), so I'd also be glad if anyone (or namely @horacio ) could help.

@ankane
Copy link
Contributor

ankane commented Jan 3, 2021

Hey @Yuki-Inoue, love this feature. Here's a working test case for it (test/kernel_test.rb). Not sure if there's a better way to generate a fake config file for testing.

require 'test_helper'
require 'json'

module IRubyTest
  class KernelTest < TestBase
    def test_after_intialize_hooks
      ran_hook = false
      IRuby::Kernel.after_initialize do
        ran_hook = true
      end

      # may be better to put this in test/test_helper.rb
      IRuby.logger ||= Logger.new(nil)

      IRuby::Kernel.new(config_file.path)

      assert ran_hook
    ensure
      IRuby::Kernel.after_initialize_hooks.clear
    end

    def config_file
      config_file = Tempfile.new
      config_file.write({key: ""}.to_json)
      config_file.flush
      config_file
    end
  end
end

@Yuki-Inoue
Copy link
Contributor Author

@kojix2 @mrkn (to maintainers)

I've added the test from @ankane .
Could you check if this is enough?

@mrkn
Copy link
Contributor

mrkn commented Jan 3, 2021

@Yuki-Inoue @ankane Thank you for your works. I'll look at it.

@mrkn mrkn self-assigned this Mar 25, 2021
@mrkn mrkn added this to the Version 0.6.0 milestone Mar 25, 2021
@mrkn
Copy link
Contributor

mrkn commented May 24, 2021

@Yuki-Inoue I wrote event manager in #295. I'd like to introduce after_initialize event by using this event manager. I'll do it in another pull request, and I'll credit you as co-author in the commit message. Thank you for your contribution, and I'm very sorry for the late response.

mrkn added a commit that referenced this pull request May 24, 2021
[Fixes GH-168]

Co-authored-by: Yuki INOUE <inoueyuworks@gmail.com>
@mrkn mrkn mentioned this pull request May 24, 2021
@mrkn mrkn closed this in eaf4b02 May 24, 2021
@Yuki-Inoue Yuki-Inoue deleted the after-initialize-hook branch May 28, 2021 04:13
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request May 29, 2021
0.7.0 (2021-05-28)

Enhancements
* The default backend is changed to IRB (@mrkn)
* Add IRuby::Kernel#switch_backend! method (@mrkn)

Bug Fixes
* Fix the handling of image/svg+xml
  SciRuby/iruby#300,
  SciRuby/iruby#301 (@kojix2)

0.6.1 (2021-05-26)

Bug Fixes
* Follow the messages and hooks orders during execute_request processing
  (@mrkn)

0.6.0 (2021-05-25)

Bug Fixes
* Fix the handling of application/javascript
  SciRuby/iruby#292,
  SciRuby/iruby#294 (@kylekyle, @mrkn)

Enhancements
* Add the initialized event in IRuby::Kernel class
  SciRuby/iruby#168,
  SciRuby/iruby#296 (@Yuki-Inoue, @mrkn)
* Add the following four events SciRuby/iruby#295
  (@mrkn):
  - pre-execute -- occurs before every code execution
  - pre-run-cell -- occurs before every non-silent code execution
  - post-execute -- occurs after every code execution
  - post-run-cell -- occurs after every non-silent code execution
* Replace Bond with IRB in PlainBackend
  SciRuby/iruby#276,
  SciRuby/iruby#297 (@cfis, @mrkn)
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

Successfully merging this pull request may close these issues.

5 participants