Skip to content

Commit 9f1e961

Browse files
authored
Disable driver loading check (#30)
As stated in the code comment, this check gives spurious errors for drivers which use Requires (or Base.package_callbacks) to load their DataSets integration code on demand.
1 parent 195d0b6 commit 9f1e961

File tree

1 file changed

+27
-10
lines changed

1 file changed

+27
-10
lines changed

src/DataSets.jl

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -638,10 +638,6 @@ end
638638

639639
function add_storage_driver(project::AbstractDataProject)
640640
for conf in data_drivers(project)
641-
if conf["type"] != "storage"
642-
# Anticipate there might be layer drivers too
643-
continue
644-
end
645641
pkgid = PkgId(UUID(conf["module"]["uuid"]), conf["module"]["name"])
646642
if Base.haskey(Base.package_locks, pkgid)
647643
# Hack: Avoid triggering another call to require() for packages
@@ -652,14 +648,35 @@ function add_storage_driver(project::AbstractDataProject)
652648
continue
653649
end
654650
mod = Base.require(pkgid)
655-
driver_name = conf["name"]
656-
# Module itself does add_storage_driver() inside its __init__
657-
# TODO: Is this a good workflow?
658-
lock(_storage_drivers_lock) do
659-
get(_storage_drivers, driver_name) do
660-
error("Package $pkgid did not provide storage driver $driver_name")
651+
#=
652+
# TODO: Improve driver loading invariants.
653+
#
654+
# The difficulty here is that there's two possible ways for drivers to
655+
# work:
656+
# 1. The driver depends explicitly on `using DataSets`, so
657+
# DataSets.__init__ is called *before* the Driver.__init__.
658+
# 2. The driver uses a Requires-like mechanism to support multiple
659+
# incompatible DataSets versions, so Driver.__init__ can occur
660+
# *before* DataSets.__init__.
661+
#
662+
# This makes it hard for DataSets to check which drivers are added by a
663+
# module: In case (2), the following check fails when the driver is
664+
# loaded before DataSets and in case (1) we hit the double-require
665+
# problem, resulting in the Base.package_locks bailout which disables
666+
# the check below.
667+
#
668+
if conf["type"] == "storage"
669+
driver_name = conf["name"]
670+
# `mod` is assumed to run add_storage_driver() inside its __init__,
671+
# unless the symbol mod.datasets_load_hook exists (in which case we
672+
# call this instead).
673+
lock(_storage_drivers_lock) do
674+
get(_storage_drivers, driver_name) do
675+
error("Package $pkgid did not provide storage driver $driver_name")
676+
end
661677
end
662678
end
679+
=#
663680
end
664681
end
665682

0 commit comments

Comments
 (0)