Skip to content
Open
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
121 changes: 119 additions & 2 deletions extensions/gubbins/gubbins-db/src/gubbins/db/sql/jobs/schema.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,134 @@
from diracx.db.sql.job.db import JobDBBase
from diracx.db.sql.utils import Column
from diracx.db.sql.job.schema import AccountedFlagEnum, InputData
from diracx.db.sql.utils import Column, EnumBackedBool, NullColumn
from diracx.db.sql.utils.types import SmarterDateTime
from sqlalchemy import (
ForeignKey,
Index,
Integer,
String,
Text,
)

# NOTE: We need to remove the original Jobs table from metadata because
# GubbinsJobs modifies an existing column (job_name size from 128 to 512).
# Simple inheritance works fine for ADDING columns (see GubbinsInputData below),
# but MODIFYING existing columns in SQLAlchemy requires replacing the table definition.
Copy link
Member

Choose a reason for hiding this comment

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

MODIFYING existing columns

IIRC we had said that we didn't want to support this, do you have a use case in mind?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Personally I do not have a use case for this, but such an example was asked by CMS.

Copy link
Contributor

Choose a reason for hiding this comment

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

We explicitly said we would not do that because it has too many implications. We will even have to modify some LHCb database to follow that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't (nor @aldbr , and probably no-one can) remember that this was said, and it is not in https://diracx.diracgrid.org/en/latest/dev/explanations/extensions/

So, this explicitly shows that at the moment this is the way to go if someone wants to do this.

Copy link
Contributor

Choose a reason for hiding this comment

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

We explicitly said we would not do that because it has too many implications. We will even have to modify some LHCb database to follow that.

Is that written anywhere in the documentation?
I found that block of documentation about DB extensions, but it's not explicitly mentioned: https://diracx.diracgrid.org/en/latest/dev/explanations/extensions/#extended-db

If not written anywhere, I think we should clearly mention it there (this section of the documentation is probably the right place)

(In the future, as we discussed, this should likely go through a Github Issue for discussion first)

# If we don't remove the original, we get duplicate table/index conflicts.
if "Jobs" in JobDBBase.metadata.tables:
JobDBBase.metadata.remove(JobDBBase.metadata.tables["Jobs"])


# Create a new external table for Gubbins-specific categories
class GubbinsCategories(JobDBBase):
"""External table for Gubbins-specific categories"""

__tablename__ = "GubbinsCategories"

category_id = Column("CategoryID", Integer, primary_key=True, autoincrement=True)
category_name = Column("CategoryName", String(255), nullable=False)
description = Column("Description", Text, default="")

__table_args__ = (Index("CategoryName", "CategoryName"), {"extend_existing": True})


# GubbinsJobs: Replaces the Jobs table with modifications
# NOTE: We cannot use simple inheritance (class GubbinsJobs(Jobs)) because we're
# modifying an existing column (job_name). SQLAlchemy inheritance works for ADDING
# columns but not for MODIFYING them. So we must redefine the entire table.
class GubbinsJobs(JobDBBase):
"""Custom Jobs table with modified column specifications and foreign key to GubbinsCategories"""

__tablename__ = "Jobs"
__table_args__ = (
Index("JobType", "JobType"),
Index("JobGroup", "JobGroup"),
Index("Site", "Site"),
Index("Owner", "Owner"),
Index("OwnerGroup", "OwnerGroup"),
Index("Status", "Status"),
Index("MinorStatus", "MinorStatus"),
Index("ApplicationStatus", "ApplicationStatus"),
Index("StatusSite", "Status", "Site"),
Index("LastUpdateTime", "LastUpdateTime"),
{"extend_existing": True},
)

# All columns from the parent Jobs table (copied from diracx.db.sql.job.schema)
job_id = Column(
"JobID",
Integer,
ForeignKey("JobJDLs.JobID", ondelete="CASCADE"),
primary_key=True,
default=0,
)
job_type = Column("JobType", String(32), default="user")
job_group = Column("JobGroup", String(32), default="00000000")
site = Column("Site", String(100), default="ANY")

# MODIFIED: job_name with VARCHAR(512) instead of VARCHAR(128)
job_name = Column("JobName", String(512), default="Unknown")

owner = Column("Owner", String(64), default="Unknown")
owner_group = Column("OwnerGroup", String(128), default="Unknown")
vo = Column("VO", String(32))
submission_time = NullColumn(
"SubmissionTime",
SmarterDateTime(),
)
reschedule_time = NullColumn(
"RescheduleTime",
SmarterDateTime(),
)
last_update_time = NullColumn(
"LastUpdateTime",
SmarterDateTime(),
)
start_exec_time = NullColumn(
"StartExecTime",
SmarterDateTime(),
)
heart_beat_time = NullColumn(
"HeartBeatTime",
SmarterDateTime(),
)
end_exec_time = NullColumn(
"EndExecTime",
SmarterDateTime(),
)
status = Column("Status", String(32), default="Received")
minor_status = Column("MinorStatus", String(128), default="Unknown")
application_status = Column("ApplicationStatus", String(255), default="Unknown")
user_priority = Column("UserPriority", Integer, default=0)
reschedule_counter = Column("RescheduleCounter", Integer, default=0)
verified_flag = Column("VerifiedFlag", EnumBackedBool(), default=False)
accounted_flag = Column("AccountedFlag", AccountedFlagEnum(), default=False)

# NEW: foreign key constraint to GubbinsCategories table
category_id = NullColumn(
"CategoryID", Integer, ForeignKey("GubbinsCategories.CategoryID")
)


# GubbinsInputData: Extends InputData by ADDING a column
# NOTE: Simple inheritance works here because we're only ADDING a new column,
# not modifying an existing one. This is the preferred approach when possible.
class GubbinsInputData(InputData):
"""Extended InputData table with Adler checksum support"""

__tablename__ = "InputData"
__table_args__ = {"extend_existing": True}

# New column for Adler checksum
adler_checksum = Column("AdlerChecksum", String(75), nullable=True)


# You need to inherit from the declarative_base of the parent DB
class GubbinsInfo(JobDBBase):
"""An extra table with respect to Vanilla diracx JobDB"""

__tablename__ = "GubbinsJobs"
__tablename__ = "GubbinsInfo"
__table_args__ = {"extend_existing": True}

job_id = Column(
"JobID", Integer, ForeignKey("Jobs.JobID", ondelete="CASCADE"), primary_key=True
Expand Down
163 changes: 78 additions & 85 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,13 @@ requires-python = ">=3.11"
keywords = []
license = { text = "GPL-3.0-only" }
classifiers = [
"Intended Audience :: Science/Research",
"License :: OSI Approved :: GNU General Public License v3 (GPLv3)",
"Programming Language :: Python :: 3",
"Topic :: Scientific/Engineering",
"Topic :: System :: Distributed Computing",
]
dependencies = [
"diracx-api",
"diracx-cli",
"diracx-client",
"diracx-core",
"Intended Audience :: Science/Research",
"License :: OSI Approved :: GNU General Public License v3 (GPLv3)",
"Programming Language :: Python :: 3",
"Topic :: Scientific/Engineering",
"Topic :: System :: Distributed Computing",
]
dependencies = ["diracx-api", "diracx-cli", "diracx-client", "diracx-core"]
dynamic = ["version"]

[project.optional-dependencies]
Expand All @@ -35,43 +30,41 @@ bypass-selection = true

[tool.ruff]
src = ["diracx-*/src", "diracx-*/tests"]
exclude = [
"diracx-client/src/diracx/client",
]
exclude = ["diracx-client/src/diracx/client"]

[tool.ruff.lint]
select = [
"E", # pycodestyle errors
"F", # pyflakes
"B", # flake8-bugbear
"I", # isort
"PLE", # pylint errors
"D", # pydocstyle
# "UP", # pyUpgrade
"FLY", # flynt
"DTZ", # flake8-datetimez
"S", # flake8-bandit
"N", # pep8-naming
"E", # pycodestyle errors
"F", # pyflakes
"B", # flake8-bugbear
"I", # isort
"PLE", # pylint errors
"D", # pydocstyle
# "UP", # pyUpgrade
"FLY", # flynt
"DTZ", # flake8-datetimez
"S", # flake8-bandit
"N", # pep8-naming
]
ignore = [
"B905",
"B008",
"B006",
"S101", # bandit: use of assert https://docs.astral.sh/ruff/rules/assert/
"D203",
"D213",
# TODO: Maybe enable these
"D100",
"D101",
"D102",
"D103",
"D104",
"D105",
"D107",
# TODO: These should be re-enabled after fixing
"D205",
"D401",
"D404",
"B905",
"B008",
"B006",
"S101", # bandit: use of assert https://docs.astral.sh/ruff/rules/assert/
"D203",
"D213",
# TODO: Maybe enable these
"D100",
"D101",
"D102",
"D103",
"D104",
"D105",
"D107",
# TODO: These should be re-enabled after fixing
"D205",
"D401",
"D404",
]

[tool.ruff.lint.isort]
Expand All @@ -88,11 +81,11 @@ required-imports = ["from __future__ import annotations"]
[tool.ruff.lint.flake8-bugbear]
# Allow default arguments like, e.g., `data: List[str] = fastapi.Query(None)`.
extend-immutable-calls = [
"fastapi.Depends",
"fastapi.Query",
"fastapi.Path",
"fastapi.Body",
"fastapi.Header",
"fastapi.Depends",
"fastapi.Query",
"fastapi.Path",
"fastapi.Body",
"fastapi.Header",
]

[tool.ruff.lint.pycodestyle]
Expand All @@ -108,36 +101,34 @@ profile = "black"

[tool.codespell]
skip = [
"diracx-client/src/diracx/client/_generated/*",
"diracx-[a-z]*/tests/*",
"diracx-testing/*",
"extensions/gubbins/gubbins-client/src/gubbins/client/_generated/*",
"extensions/gubbins/gubbins-*/tests/*",
]
ignore-words-list = [
"CheckIn",
"dependant",
"diracx-client/src/diracx/client/_generated/*",
"diracx-[a-z]*/tests/*",
"diracx-testing/*",
"extensions/gubbins/gubbins-client/src/gubbins/client/_generated/*",
"extensions/gubbins/gubbins-*/tests/*",
]
ignore-words-list = ["CheckIn", "dependant"]


[tool.mypy]
python_version = "3.11"
files = [
"diracx-api/src/**/*.py",
"diracx-cli/src/**/*.py",
"diracx-client/src/**/_patch.py",
"diracx-core/src/**/*.py",
"diracx-db/src/**/*.py",
"diracx-logic/src/**/*.py",
"diracx-routers/src/**/*.py",
"diracx-api/src/**/*.py",
"diracx-cli/src/**/*.py",
"diracx-client/src/**/_patch.py",
"diracx-core/src/**/*.py",
"diracx-db/src/**/*.py",
"diracx-logic/src/**/*.py",
"diracx-routers/src/**/*.py",
]
mypy_path = [
"$MYPY_CONFIG_FILE_DIR/diracx-api/src",
"$MYPY_CONFIG_FILE_DIR/diracx-cli/src",
"$MYPY_CONFIG_FILE_DIR/diracx-client/src",
"$MYPY_CONFIG_FILE_DIR/diracx-core/src",
"$MYPY_CONFIG_FILE_DIR/diracx-db/src",
"$MYPY_CONFIG_FILE_DIR/diracx-logic/src",
"$MYPY_CONFIG_FILE_DIR/diracx-routers/src",
"$MYPY_CONFIG_FILE_DIR/diracx-api/src",
"$MYPY_CONFIG_FILE_DIR/diracx-cli/src",
"$MYPY_CONFIG_FILE_DIR/diracx-client/src",
"$MYPY_CONFIG_FILE_DIR/diracx-core/src",
"$MYPY_CONFIG_FILE_DIR/diracx-db/src",
"$MYPY_CONFIG_FILE_DIR/diracx-logic/src",
"$MYPY_CONFIG_FILE_DIR/diracx-routers/src",
]
plugins = ["sqlalchemy.ext.mypy.plugin", "pydantic.mypy"]
allow_redefinition = true
Expand All @@ -160,23 +151,25 @@ log_cli_level = "INFO"
xfail_strict = true
filterwarnings = ["default"]
testpaths = [
"diracx-api/tests",
"diracx-cli/tests",
"diracx-client/tests",
"diracx-core/tests",
"diracx-db/tests",
"diracx-routers/tests",
"diracx-api/tests",
"diracx-cli/tests",
"diracx-client/tests",
"diracx-core/tests",
"diracx-db/tests",
"diracx-routers/tests",
]
addopts = [
"-v",
"--cov=diracx",
"--cov-report=term-missing",
"-pdiracx.testing",
"-pdiracx.testing.osdb",
"--import-mode=importlib",
"-ra", "--strict-config", "--strict-markers",
"-v",
"--cov=diracx",
"--cov-report=term-missing",
"-pdiracx.testing",
"-pdiracx.testing.osdb",
"--import-mode=importlib",
"-ra",
"--strict-config",
"--strict-markers",
]
asyncio_mode = "auto"
markers = [
"enabled_dependencies: List of dependencies which should be available to the FastAPI test client",
"enabled_dependencies: List of dependencies which should be available to the FastAPI test client",
]
Loading