Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 22 additions & 10 deletions simple/stats/jsonld_exporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,17 +140,23 @@ def _process_observation_chunk(args):
This runs in a separate process, so it must establish its own DB connection
and import necessary modules locally.
"""
shard_index, offset, chunk_size, db_path, output_dir_path, ns_map, prov_urls = args
shard_index, offset, chunk_size, db_path, output_dir_path, ns_map, prov_urls, provenance = args

# Open a new connection for this worker process (SQLite connections cannot be shared across processes)
conn = sqlite3.connect(db_path)
cursor = conn.cursor()

# Fetch the specific chunk of observations for this shard
cursor.execute(
"SELECT entity, variable, date, value, provenance, unit, scaling_factor, "
"measurement_method, observation_period, properties FROM observations LIMIT ? OFFSET ?",
(chunk_size, offset))
if provenance:
cursor.execute(
"SELECT entity, variable, date, value, provenance, unit, scaling_factor, "
"measurement_method, observation_period, properties FROM observations WHERE provenance = ? LIMIT ? OFFSET ?",
(provenance, chunk_size, offset))
else:
cursor.execute(
"SELECT entity, variable, date, value, provenance, unit, scaling_factor, "
"measurement_method, observation_period, properties FROM observations LIMIT ? OFFSET ?",
(chunk_size, offset))
obs_tuples = cursor.fetchall()
conn.close()

Expand All @@ -173,13 +179,17 @@ def _process_observation_chunk(args):
return True


def process_observations(db, output_dir, ns_map: dict, chunk_size: int):
def process_observations(db, output_dir, ns_map: dict, chunk_size: int, provenance: str = None):
"""Processes observations in chunks in parallel and writes them to JSON-LD shards."""
db_path = db.engine.db_file.syspath()
output_dir_path = output_dir.full_path()

# Calculate the total number of chunks needed
total_obs = db.engine.fetch_all("SELECT COUNT(*) FROM observations")[0][0]
if provenance:
total_obs = db.engine.fetch_all("SELECT COUNT(*) FROM observations WHERE provenance = ?", (provenance,))[0][0]
else:
total_obs = db.engine.fetch_all("SELECT COUNT(*) FROM observations")[0][0]

num_chunks = (total_obs + chunk_size - 1) // chunk_size

# Fetch all provenance URLs once to pass to workers
Expand All @@ -190,7 +200,7 @@ def process_observations(db, output_dir, ns_map: dict, chunk_size: int):

# Prepare arguments for the worker pool (each chunk gets its own offset and index)
args_list = [(i, i * chunk_size, chunk_size, db_path, output_dir_path, ns_map,
prov_urls) for i in range(num_chunks)]
prov_urls, provenance) for i in range(num_chunks)]

# Cap the number of processes to avoid overloading the machine
num_processes = min(multiprocessing.cpu_count(), 8)
Expand All @@ -205,7 +215,8 @@ def process_observations(db, output_dir, ns_map: dict, chunk_size: int):
def export_to_jsonld(db,
output_dir,
chunk_size: int = 10000,
context: dict = None):
context: dict = None,
provenance: str = None):
"""Exports resolved data from the database to JSON-LD shards.

Args:
Expand All @@ -214,6 +225,7 @@ def export_to_jsonld(db,
output_dir: The directory where JSON-LD shards will be written.
chunk_size: The number of rows to fetch and process at a time.
context: Optional custom JSON-LD context mappings.
provenance: Optional provenance filter for observations.
"""
logging.info("Exporting resolved data to JSON-LD in shards")

Expand All @@ -225,7 +237,7 @@ def export_to_jsonld(db,
process_triples(db, output_dir, ns_map, chunk_size)

# 2. Process Observations in chunks
process_observations(db, output_dir, ns_map, chunk_size)
process_observations(db, output_dir, ns_map, chunk_size, provenance)


def write_shard(g: Graph,
Expand Down
18 changes: 16 additions & 2 deletions simple/stats/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
"The input directory.")
flags.DEFINE_string("output_dir", constants.DEFAULT_OUTPUT_DIR,
"The output directory.")
flags.DEFINE_string("import_name", None,
"The name of the import (subdirectory under input_dir).")
flags.DEFINE_enum(
"mode",
RunMode.CUSTOM_DC,
Expand All @@ -54,10 +56,22 @@
def _run():
initialize_logger()
logging.info("Starting stats data importer job in mode: %s", FLAGS.mode)

input_dir = FLAGS.input_dir
if FLAGS.import_name == "ALL_IMPORTS":
logging.info("Running bulk load for all imports under: %s", input_dir)
elif FLAGS.import_name and "," in FLAGS.import_name:
logging.info("Running combined load for specific imports: %s", FLAGS.import_name)
elif FLAGS.import_name:
import os
input_dir = os.path.join(input_dir, FLAGS.import_name)
logging.info("Using import specific directory: %s", input_dir)

Runner(config_file_path=FLAGS.config_file,
input_dir_path=FLAGS.input_dir,
input_dir_path=input_dir,
output_dir_path=FLAGS.output_dir,
mode=FLAGS.mode).run()
mode=FLAGS.mode,
import_name=FLAGS.import_name).run()
logging.info("Runner finished successfully.")


Expand Down
157 changes: 140 additions & 17 deletions simple/stats/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
from stats.entities_importer import EntitiesImporter
from stats.events_importer import EventsImporter
from stats.importer import Importer
from stats.jsonld_exporter import export_to_jsonld
from stats.jsonld_exporter import DCID_URL, export_to_jsonld, process_observations, process_triples
from stats.mcf_importer import McfImporter
import stats.nl as nl
from stats.nodes import Nodes
Expand All @@ -70,6 +70,9 @@ class RunMode(StrEnum):
DCP_BRIDGE = "dcpbridge"


_ARCHIVES_DIR_NAME = "archives"


class Runner:
"""Runs and coordinates all imports."""

Expand All @@ -79,12 +82,14 @@ def __init__(
input_dir_path: str,
output_dir_path: str,
mode: RunMode = RunMode.CUSTOM_DC,
import_name: Optional[str] = None,
) -> None:
assert (config_file_path or
input_dir_path), "One of config_file or input_dir must be specified"
assert output_dir_path, "output_dir must be specified"

self.mode = mode
self.import_name = import_name

# File systems, both input and output. Must be closed when run finishes.
self.all_stores: list[Store] = []
Expand Down Expand Up @@ -114,10 +119,19 @@ def __init__(
self.all_stores.append(input_store)
self.input_stores.append(input_store)

self._read_config_from_file(
config_file_path=constants.CONFIG_JSON_FILE_NAME,
config_file_dir=input_store.as_dir(),
)
if self.import_name == "ALL_IMPORTS":
self._read_configs_from_subdirs(input_store.as_dir())
elif self.import_name and "," in self.import_name:
self._read_configs_from_list(input_store.as_dir(), self.import_name.split(","))
else:
try:
self._read_config_from_file(
config_file_path=constants.CONFIG_JSON_FILE_NAME,
config_file_dir=input_store.as_dir(),
)
except FileNotFoundError:
logging.info("Config file not found at root of %s. Scanning subdirectories.", input_dir_path)
self._read_configs_from_subdirs(input_store.as_dir())

# Get dict of special file type string to special file name.
# Example entry: verticalSpecsFile -> vertical_specs.json
Expand Down Expand Up @@ -212,6 +226,85 @@ def _read_config_from_file(self,
config_data = json.loads(raw_config) if raw_config else {}
self.config = Config(data=config_data)

def _merge_configs(self, configs: list, base_dir: Dir):
import json
import fs.path as fspath

merged_data = {
"importName": "ALL_IMPORTS",
"includeInputSubdirs": True,
"inputFiles": {},
"variables": {},
"sources": {}
}

for file in configs:
raw_config = file.read()
try:
config_data = json.loads(raw_config)
except json.JSONDecodeError as e:
logging.error("Failed to parse JSON from %s: %s", file.full_path(), e)
raise e
Comment on lines +243 to +247

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If a config.json file contains valid JSON but is not a JSON object (e.g., a JSON array or a primitive value), json.loads will succeed but subsequent .get() calls on config_data will raise an AttributeError. We should defensively verify that the parsed JSON is a dictionary.

Suggested change
try:
config_data = json.loads(raw_config)
except json.JSONDecodeError as e:
logging.error("Failed to parse JSON from %s: %s", file.full_path(), e)
raise e
try:
config_data = json.loads(raw_config)
if not isinstance(config_data, dict):
raise ValueError("Config content must be a JSON object")
except (json.JSONDecodeError, ValueError) as e:
logging.error("Failed to parse JSON from %s: %s", file.full_path(), e)
raise e


dir_path = fspath.dirname(file.path)
rel_dir = dir_path.replace(base_dir.path, "").strip("/")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Using str.replace to remove the base directory path from dir_path is highly error-prone. If the base directory name (e.g., base or import) appears anywhere else in the subdirectory path (for example, /base/import/my_import/base/config.json), replace will remove all occurrences of that substring, corrupting the relative path. Use fs.path.relative to safely compute the relative path.

Suggested change
rel_dir = dir_path.replace(base_dir.path, "").strip("/")
rel_dir = fspath.relative(base_dir.path, dir_path).strip("/")


logging.info("Merging config from import directory: %s", rel_dir)

# Merge inputFiles, prefixing keys with rel_dir
input_files = config_data.get("inputFiles", {})
for k, v in input_files.items():
new_key = fspath.join(rel_dir, k)
merged_data["inputFiles"][new_key] = v

# Merge variables
variables = config_data.get("variables", {})
for k, v in variables.items():
merged_data["variables"][k] = v

# Merge sources
sources = config_data.get("sources", {})
for k, v in sources.items():
merged_data["sources"][k] = v
Comment on lines +260 to +268

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

When merging variables and sources from multiple configuration files, conflicting definitions for the same key will silently overwrite each other. To prevent hard-to-debug configuration conflicts, we should log a warning if a key is being overwritten with a different value.

Suggested change
# Merge variables
variables = config_data.get("variables", {})
for k, v in variables.items():
merged_data["variables"][k] = v
# Merge sources
sources = config_data.get("sources", {})
for k, v in sources.items():
merged_data["sources"][k] = v
# Merge variables, warning on conflicts
variables = config_data.get("variables", {})
for k, v in variables.items():
if k in merged_data["variables"] and merged_data["variables"][k] != v:
logging.warning("Conflicting definition for variable %s. Overwriting.", k)
merged_data["variables"][k] = v
# Merge sources, warning on conflicts
sources = config_data.get("sources", {})
for k, v in sources.items():
if k in merged_data["sources"] and merged_data["sources"][k] != v:
logging.warning("Conflicting definition for source %s. Overwriting.", k)
merged_data["sources"][k] = v


self.config = Config(data=merged_data)

def _find_configs_in_dir(self, directory: Dir) -> list:
"""Finds all config.json files in a directory, excluding archives."""
configs = []
for file in directory.all_files(include_subdirs=True):
if _ARCHIVES_DIR_NAME in file.path.split("/"):
continue
if file.name() == constants.CONFIG_JSON_FILE_NAME:
configs.append(file)
return configs

def _read_configs_from_subdirs(self, base_dir: Dir):
configs = self._find_configs_in_dir(base_dir)
logging.info("Found %s config files in subdirectories.", len(configs))
self._merge_configs(configs, base_dir)

def _read_configs_from_list(self, base_dir: Dir, import_names: list[str]):
configs = []
for name in import_names:
name = name.strip()
Comment on lines +289 to +290

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If import_names contains empty strings or trailing commas (e.g., "oecd,"), name.strip() will be empty. Calling base_dir.open_dir("") on an empty string will open the base directory itself, which is unintended. We should skip empty names.

    for name in import_names:
      name = name.strip()
      if not name:
        continue

try:
imp_dir = base_dir.open_dir(name)
file = imp_dir.open_file(constants.CONFIG_JSON_FILE_NAME, create_if_missing=False)
configs.append(file)
except FileNotFoundError:
logging.info("Config file not found at root of %s. Scanning subdirectories.", name)
sub_configs = self._find_configs_in_dir(imp_dir)
if not sub_configs:
raise FileNotFoundError(f"No config files found for {name}")
configs.extend(sub_configs)
except ValueError as e:
logging.error("Invalid directory for import %s: %s", name, e)
raise e
Comment on lines +291 to +303

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If base_dir.open_dir(name) fails or raises an exception before imp_dir is assigned, referencing imp_dir in the except FileNotFoundError block will raise an UnboundLocalError. We should separate the directory opening from the file opening to ensure imp_dir is safely bound before it is used.

Suggested change
try:
imp_dir = base_dir.open_dir(name)
file = imp_dir.open_file(constants.CONFIG_JSON_FILE_NAME, create_if_missing=False)
configs.append(file)
except FileNotFoundError:
logging.info("Config file not found at root of %s. Scanning subdirectories.", name)
sub_configs = self._find_configs_in_dir(imp_dir)
if not sub_configs:
raise FileNotFoundError(f"No config files found for {name}")
configs.extend(sub_configs)
except ValueError as e:
logging.error("Invalid directory for import %s: %s", name, e)
raise e
try:
imp_dir = base_dir.open_dir(name)
except (FileNotFoundError, ValueError) as e:
logging.error("Invalid or missing directory for import %s: %s", name, e)
raise e
try:
file = imp_dir.open_file(constants.CONFIG_JSON_FILE_NAME, create_if_missing=False)
configs.append(file)
except FileNotFoundError:
logging.info("Config file not found at root of %s. Scanning subdirectories.", name)
sub_configs = self._find_configs_in_dir(imp_dir)
if not sub_configs:
raise FileNotFoundError(f"No config files found for {name}")
configs.extend(sub_configs)


logging.info("Found %s config files from list.", len(configs))
self._merge_configs(configs, base_dir)

def _get_db_config(self) -> dict:
if self.mode == RunMode.MAIN_DC:
logging.info("Using Main DC config.")
Expand Down Expand Up @@ -481,6 +574,8 @@ def _run_all_data_imports(self):
input_files.append(input_store.as_file())

for input_file in input_files:
if _ARCHIVES_DIR_NAME in input_file.path.split("/"):
continue
if self._check_if_special_file(input_file):
continue
if match(input_file, "*.csv"):
Expand All @@ -494,6 +589,7 @@ def _run_all_data_imports(self):

logging.info(f"Found {len(input_csv_files)} csv files to import")
logging.info(f"Found {len(input_mcf_files)} mcf files to import")
logging.info("Matched files to process: %s", [f.full_path() for f in input_csv_files + input_mcf_files])

self.reporter.report_started(import_files=list(input_csv_files +
input_mcf_files))
Expand Down Expand Up @@ -596,21 +692,48 @@ def _run_imports_and_export_jsonld(self):

# Export to JSON-LD
jsonld_dir = self.output_dir.open_dir("jsonld")

# Create a unique subfolder based on import name and timestamp for parallel runs
import_name = self.config.data.get("importName") or "default_import_name"

# Get all unique provenances from observations
rows = self.db.engine.fetch_all("SELECT DISTINCT provenance FROM observations")
provenances = [row[0] for row in rows if row[0]]
Comment on lines +697 to +698

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If any observations have a missing or empty provenance field, they will be completely skipped during the export because provenances only includes non-empty values. To prevent silent data loss, we should log a warning if any observations with an empty provenance are detected.

Suggested change
rows = self.db.engine.fetch_all("SELECT DISTINCT provenance FROM observations")
provenances = [row[0] for row in rows if row[0]]
rows = self.db.engine.fetch_all("SELECT DISTINCT provenance FROM observations")
provenances = [row[0] for row in rows if row[0]]
if any(not row[0] for row in rows):
logging.warning("Found observations with missing or empty provenance. These will be skipped during export!")


logging.info("Found provenances to export: %s", provenances)

timestamp = datetime.now(timezone.utc).strftime("%Y%m%d_%H%M%S_%f")
unique_dir_name = f"{import_name}_{timestamp}"
unique_jsonld_dir = jsonld_dir.open_dir(unique_dir_name)

ns_map = {"dcid": DCID_URL}

self.db.commit()
export_to_jsonld(self.db, unique_jsonld_dir)


# Export triples (schema) to a common folder
schema_dir = jsonld_dir.open_dir(f"schema_{timestamp}")
process_triples(self.db, schema_dir, ns_map, chunk_size=10000)

import_list = []
# Add schema to import list
import_list.append({
"importName": "schema",
"graphPath": f"{schema_dir.full_path().rstrip('/')}/*.jsonld"
})

# Export observations per provenance
for prov in provenances:
# Sanitize provenance name for folder
prov_folder = prov.replace("/", "_").replace(":", "_")
Comment on lines +720 to +721

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Sanitizing the provenance name by only replacing / and : is not fully robust. Provenance strings (especially URLs) can contain other characters like ?, &, =, or \, which are invalid or problematic in directory names on various filesystems (e.g., Windows). Using a regular expression to replace all non-alphanumeric characters (except underscores and dashes) is much safer and more robust.

Suggested change
# Sanitize provenance name for folder
prov_folder = prov.replace("/", "_").replace(":", "_")
# Sanitize provenance name for folder to be safe for all filesystems
import re
prov_folder = re.sub(r'[^a-zA-Z0-9_\-]', '_', prov)

prov_dir = jsonld_dir.open_dir(f"{prov_folder}_{timestamp}")
process_observations(self.db, prov_dir, ns_map, chunk_size=10000, provenance=prov)

import_list.append({
"importName": prov,
"graphPath": f"{prov_dir.full_path().rstrip('/')}/*.jsonld"
})

# Auto-trigger workflow if output is on GCS
output_path = unique_jsonld_dir.full_path()
if os.getenv("INGESTION_WORKFLOW_NAME") and output_path.startswith("gs://"):
gcs_pattern = f"{output_path.rstrip('/')}/*.jsonld"
trigger_ingestion_workflow(gcs_pattern, import_name)
import_name = self.import_name or self.config.data.get("importName") or "default_import_name"
if import_name and "/" in import_name:
import_name = import_name.replace("/", "_")

if os.getenv("INGESTION_WORKFLOW_NAME") and jsonld_dir.full_path().startswith("gs://"):
trigger_ingestion_workflow(import_list, import_name)
else:
logging.info(
"Output is local or workflow is missing, skipping auto-trigger of ingestion workflow. Please upload files to GCS and trigger manually."
Expand Down
7 changes: 2 additions & 5 deletions simple/stats/trigger_ingestion_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def _get_env_vars():
return {var: os.getenv(var) for var in required_env_vars}


def trigger_ingestion_workflow(gcs_path: str,
def trigger_ingestion_workflow(import_list: list,
import_name: str = "default_import_name"):
"""Triggers the Data Commons ingestion workflow via Google Cloud Workflows API."""
logging.info("Attempting to auto-trigger ingestion workflow via API...")
Expand All @@ -59,10 +59,7 @@ def trigger_ingestion_workflow(gcs_path: str,
"importName":
import_name,
"importList":
json.dumps([{
"importName": import_name,
"graphPath": gcs_path
}]),
json.dumps(import_list),
"tempLocation":
env_vars["TEMP_LOCATION"],
"region":
Expand Down
Loading