kimlai/tz_world

TzWorld.Backend.DetsWithIndexCache raises when .dets file doesn't exist

Closed this issue · 8 comments

I'm unable to use the TzWorld.Backend.DetsWithIndexCache backend due to the following error:

11:46:23.073 module=gen_server function=error_info/7 line=889 [error] GenServer TzWorld.Backend.DetsWithIndexCache terminating
** (MatchError) no match of right hand side value: {:error, {:file_error, '/Users/aaronross/Sites/haas/_build/test/lib/tz_world/priv/timezones-geodata.dets', :enoent}}
    (tz_world) lib/tz_world/backend/dets_with_index_cache.ex:97: TzWorld.Backend.DetsWithIndexCache.handle_continue/2
    (stdlib) gen_server.erl:637: :gen_server.try_dispatch/4
    (stdlib) gen_server.erl:388: :gen_server.loop/7
    (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3
Last message: {:continue, :open_dets_file}

This issue seems to be caused when calling :dets.open_file(__MODULE__, access: :read) when the file doesn't already exist. The file will be automatically created when :access is :read_write, but not when it's :read.

Thanks for the report, and apologies for the inconvenience. I'll get this corrected asap. What do you consider the correct behaviour to be in the case when no data is available:

  1. Silent failure (perhaps with logging) and just return an error tuple for lookups (current behaviour)
  2. Trigger a download/generation of the data automatically
  3. Trigger a download/generation of the data according to configuration
  4. Any other suggestion?

I dug into this a little further and I think I found the underlying issue - when I ran mix tz_world.update initially, that downloads the .zip file and creates the .dets file (by calling TzWorld.Backend.Dets.reload_timezone_data()) to tz_world's priv dir. Unfortunately, when I run mix test, tz_world uses a different priv dir**, so the .dets file isn't there and all lookups fail.

I think the best DX would be if each backend checked if the .dets file was present and/or empty, and if so pulled in the data from the .zip archive on load. I guess I could also just do

MIX_ENV=test mix tz_world.update

but that means I'll need to do that for staging and prod as well, and gets kinda tricky when working with CI environments too. Thoughts?

** the priv dir is _build/dev/lib/tz_world/priv for dev and _build/test/lib/tz_world/priv for test

Ahhhh. Understood. I have just added some code to be more resilient in the face of no :dets file.

What you are seeing is that the ./priv dir isn't included in the hex package. When it is included, mix will symlink it into the build directory for any environment except :prod. Which means your tests in :dev and :staging will not need you to download the data each time.

This was "fixed" in tz_world version 0.5.0 - may I ask you to check if thats the version you have installed? In that version, the compilation phase will create the deps/tz_world/priv directory before compilation and symlinking should work.

Hmm, just double-checked and I'm on 0.5.0. I am using a custom data_dir and it looks like that isn't taken into account when determining the .dets file location, perhaps that would be the easiest solution?

# dets.ex
  def filename do
    :code.priv_dir(:tz_world) ++ '/timezones-geodata.dets'
  end

Ah ha! Thats pretty sloppy of me. It was worth making the :dets stuff more resilient so thanks for that prompt. And now I'll fix the lack of support for the custom data_dir.

It might be 24 hours before I can finish, test and publish a new release. Are you able to work around this until then? If not I'll try to juggle a few priorities.

No worries! You've already been super helpful, the quick responses are appreciated. I'm just going to use a fork with the following change:

  @doc false
  def filename do
    TzWorld.GeoData.data_dir()
    |> Path.join("timezones-geodata.dets")
    |> String.to_charlist()
  end

That should keep things afloat for now, I'll leave a note to update to the official release once it's deployed.

Thanks for the help!

I have published tz_world version 0.6.0 to honour the :data_dir configuration and to make the code more resilient in the face of no :dets table. I definitely need to add some more tests around this (he says relying on the "pre 1.0 so its ok to push fast and often" excuse).

Let me know if this resolves at least your original issue?

Bug Fixes

  • Honour the configuration for :data_dir. Thanks to @superhawk610. Fixes #12

  • Be more resilient if the :dets file is not in place

That fixed it, thanks so much!