Skip to content

CASSANDRA-17684 Bundle CQL.html as Python package resource #4584

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.

Sign up for GitHub

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

Open
briangoins wants to merge 1 commit into
base: trunk
Choose a base branch
from
+236 −10
Open

CASSANDRA-17684 Bundle CQL.html as Python package resource #4584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ data/
conf/hotspot_compiler
doc/cql3/CQL.html
doc/build/
pylib/cqlshlib/resources/CQL.html
pylib/cqlshlib/resources/CQL.css
lib/
pylib/src/
**/cqlshlib.xml
Expand Down
15 changes: 14 additions & 1 deletion build.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
<property name="build.classes.main" value="${build.classes}/main" />
<property name="javadoc.dir" value="${build.dir}/javadoc"/>
<property name="interface.dir" value="${basedir}/interface"/>
<property name="pylib.resources" value="${basedir}/pylib/cqlshlib/resources"/>
<property name="test.dir" value="${basedir}/test"/>
<property name="test.resources" value="${test.dir}/resources"/>
<property name="test.lib" value="${build.dir}/test/lib"/>
Expand Down Expand Up @@ -458,6 +459,8 @@
<delete dir="${version.properties.dir}" />
<delete dir="${jacoco.export.dir}" />
<delete dir="${jacoco.partials.dir}"/>
<delete file="${pylib.resources}/CQL.html"/>
<delete file="${pylib.resources}/CQL.css"/>
</target>
<target depends="clean" name="cleanall"/>

Expand Down Expand Up @@ -506,6 +509,16 @@
</wikitext-to-html>
</target>

<target name="copy-cql-docs-to-pylib" depends="generate-cql-html"
description="Copy CQL documentation to pylib resources for packaging">
<copy todir="${pylib.resources}" failonerror="true">
<fileset dir="doc/cql3">
<include name="CQL.html"/>
<include name="CQL.css"/>
</fileset>
</copy>
</target>
Comment on lines +512 to +520
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

The copy task should ensure the target directory exists before attempting to copy files. While the directory should exist due to the init.py file in version control, it's a best practice to add mkdir to handle edge cases or if the directory structure is ever modified. Consider adding a mkdir task before the copy operation.

Copilot uses AI. Check for mistakes.

<target name="gen-asciidoc" description="Generate dynamic asciidoc pages" depends="jar" unless="ant.gen-doc.skip">
<exec executable="make" osfamily="unix" dir="${doc.dir}" failonerror="true">
<arg value="gen-asciidoc"/>
Expand Down Expand Up @@ -586,7 +599,7 @@
</javac>
</target>

<target depends="init,gen-cql3-grammar,generate-cql-html,generate-jflex-java"
<target depends="init,gen-cql3-grammar,copy-cql-docs-to-pylib,generate-jflex-java"
name="build-project">
<echo message="${ant.project.name}: ${ant.file}"/>
<!-- Order matters! -->
Expand Down
59 changes: 51 additions & 8 deletions pylib/cqlshlib/cqlshmain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2125,14 +2125,57 @@ def read_options(cmdlineargs, parser, config_file, cql_dir, environment=os.envir


def get_docspath(path):
cqldocs_url = Shell.DEFAULT_CQLDOCS_URL
if os.path.exists(path + '/doc/cql3/CQL.html'):
# default location of local CQL.html
cqldocs_url = 'file://' + path + '/doc/cql3/CQL.html'
elif os.path.exists('/usr/share/doc/cassandra/CQL.html'):
# fallback to package file
cqldocs_url = 'file:///usr/share/doc/cassandra/CQL.html'
return cqldocs_url
"""
Determine the URL for CQL documentation.

Priority order:
1. Local development/tarball path: {path}/doc/cql3/CQL.html
2. Linux package path: /usr/share/doc/cassandra/CQL.html
3. macOS path: /usr/local/share/doc/cassandra/CQL.html
4. Bundled package resource (for pip installs, etc.)
5. Online documentation URL (fallback)
"""
# Check local dev/tarball path
local_path = os.path.join(path, 'doc', 'cql3', 'CQL.html')
if os.path.exists(local_path):
return 'file://' + os.path.abspath(local_path)

# Check system package installation paths
for system_path in ['/usr/share/doc/cassandra/CQL.html',
'/usr/local/share/doc/cassandra/CQL.html']:
if os.path.exists(system_path):
return 'file://' + system_path

# Try to load from package resources
resource_url = _get_docs_from_package_resource()
if resource_url:
return resource_url

# Fall back to online documentation
return Shell.DEFAULT_CQLDOCS_URL


def _get_docs_from_package_resource():
"""
Attempt to load CQL documentation from package resources.
Returns a file:// URL to the resource, or None if unavailable.
"""
try:
if sys.version_info >= (3, 9):
from importlib.resources import files, as_file
resource = files('cqlshlib.resources').joinpath('CQL.html')
with as_file(resource) as path:
if path.exists():
return 'file://' + str(path.resolve())
else:
# Python 3.8 compatibility
from importlib.resources import path as resource_path
with resource_path('cqlshlib.resources', 'CQL.html') as path:
if path.exists():
return 'file://' + str(path.resolve())
Comment on lines +2163 to +2175
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

The context manager pattern here is problematic. The 'as_file' context manager may create temporary files that are cleaned up when exiting the context (which happens before returning). Since the returned URL will be used later when users run HELP commands, the file may no longer exist.

The proper approach is to check if the resource exists before using as_file, and ensure the resource is accessible. For Python 3.9+, you can use 'files()' to get a Traversable and check 'is_file()' on it. If it exists in a package that's already on the filesystem, as_file will return the actual path (no cleanup issue). However, if the package is in a zip file, the extracted temp file will be cleaned up immediately after return.

Consider one of these alternatives:

  1. Check resource.is_file() before using as_file, and handle the zip case differently
  2. Extract and cache the resource to a permanent location at startup
  3. Keep the context manager open for the shell's lifetime by managing it at a higher scope

Copilot uses AI. Check for mistakes.
except Exception:
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

'except' clause does nothing but pass and there is no explanatory comment.

Suggested change
except Exception:
except Exception:
# Any error while loading the bundled CQL docs is non-fatal; fall back to other locations.

Copilot uses AI. Check for mistakes.
pass
return None


def insert_driver_hooks():
Expand Down
24 changes: 24 additions & 0 deletions pylib/cqlshlib/resources/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""
Bundled resources for cqlshlib.

This package contains static resources (like CQL documentation) that are
bundled with cqlshlib for distribution as a Python package. These resources
are used as fallbacks when the documentation cannot be found in the standard
installation paths.
"""
140 changes: 140 additions & 0 deletions pylib/cqlshlib/test/test_docspath.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import os
import tempfile
from unittest.mock import patch

from .basecase import BaseTestCase
from cqlshlib.cqlshmain import get_docspath, _get_docs_from_package_resource, Shell


class TestGetDocspath(BaseTestCase):
"""
Tests for the get_docspath() function.

Verifies that CQL documentation paths are resolved according to the
function's priority logic.
"""

def test_local_dev_path(self):
"""Local doc/cql3/CQL.html takes precedence over all other paths."""
with tempfile.TemporaryDirectory() as tmpdir:
docs_dir = os.path.join(tmpdir, 'doc', 'cql3')
os.makedirs(docs_dir)
docs_file = os.path.join(docs_dir, 'CQL.html')
with open(docs_file, 'w') as f:
f.write('<html></html>')

result = get_docspath(tmpdir)

self.assertTrue(result.startswith('file://'))
self.assertIn('doc/cql3/CQL.html', result)
self.assertEqual(result, 'file://' + os.path.abspath(docs_file))

def test_linux_package_path(self):
"""Linux package path when local path doesn't exist."""
with tempfile.TemporaryDirectory() as tmpdir:
with patch('os.path.exists') as mock_exists:
def exists_side_effect(path):
if path == os.path.join(tmpdir, 'doc', 'cql3', 'CQL.html'):
return False
if path == '/usr/share/doc/cassandra/CQL.html':
return True
return False

mock_exists.side_effect = exists_side_effect

result = get_docspath(tmpdir)

self.assertEqual(result, 'file:///usr/share/doc/cassandra/CQL.html')

def test_macos_path(self):
"""macOS path when local and Linux paths don't exist."""
with tempfile.TemporaryDirectory() as tmpdir:
with patch('os.path.exists') as mock_exists:
def exists_side_effect(path):
if path == os.path.join(tmpdir, 'doc', 'cql3', 'CQL.html'):
return False
if path == '/usr/share/doc/cassandra/CQL.html':
return False
if path == '/usr/local/share/doc/cassandra/CQL.html':
return True
return False

mock_exists.side_effect = exists_side_effect

result = get_docspath(tmpdir)

self.assertEqual(result, 'file:///usr/local/share/doc/cassandra/CQL.html')

def test_package_resource(self):
"""Package resource when filesystem paths don't exist."""
with tempfile.TemporaryDirectory() as tmpdir:
with patch('os.path.exists', return_value=False):
with patch('cqlshlib.cqlshmain._get_docs_from_package_resource') as mock_resource:
mock_resource.return_value = 'file:///some/resource/path/CQL.html'

result = get_docspath(tmpdir)

self.assertEqual(result, 'file:///some/resource/path/CQL.html')
mock_resource.assert_called_once()

def test_online_url_fallback(self):
"""Online documentation URL when all local paths fail."""
with tempfile.TemporaryDirectory() as tmpdir:
with patch('os.path.exists', return_value=False):
with patch('cqlshlib.cqlshmain._get_docs_from_package_resource', return_value=None):
result = get_docspath(tmpdir)

self.assertEqual(result, Shell.DEFAULT_CQLDOCS_URL)


class TestGetDocsFromPackageResource(BaseTestCase):
"""Tests for the _get_docs_from_package_resource() function."""

def test_returns_none_on_import_error(self):
"""Should return None if importlib.resources is not available."""
with patch.dict('sys.modules', {'importlib.resources': None}):
with patch('cqlshlib.cqlshmain.sys.version_info', (3, 9)):
with patch('builtins.__import__', side_effect=ImportError):
result = _get_docs_from_package_resource()
self.assertIsNone(result)

def test_returns_none_when_resource_not_found(self):
"""Should return None if the resource file doesn't exist."""
from unittest.mock import MagicMock
from contextlib import contextmanager

@contextmanager
def mock_as_file(resource):
mock_path = MagicMock()
mock_path.exists.return_value = False
yield mock_path

with patch('cqlshlib.cqlshmain.sys.version_info', (3, 9)):
with patch('importlib.resources.files') as mock_files:
with patch('importlib.resources.as_file', mock_as_file):
mock_files.return_value.joinpath.return_value = MagicMock()
result = _get_docs_from_package_resource()
self.assertIsNone(result)

def test_exception_handling(self):
"""Should handle exceptions gracefully and return None."""
with patch('cqlshlib.cqlshmain.sys.version_info', (3, 9)):
with patch('importlib.resources.files', side_effect=Exception("Test error")):
result = _get_docs_from_package_resource()
self.assertIsNone(result)
Comment on lines +109 to +140
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

All tests in TestGetDocsFromPackageResource mock sys.version_info to (3, 9), which means the Python 3.8 compatibility code path (lines 2170-2175 in cqlshmain.py) is never tested. Add test cases that set sys.version_info to (3, 8) to ensure the Python 3.8 compatibility path works correctly.

Copilot uses AI. Check for mistakes.
6 changes: 5 additions & 1 deletion pylib/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ def get_extensions():
setup(
name="cassandra-pylib",
description="Cassandra Python Libraries",
packages=["cqlshlib"],
packages=["cqlshlib", "cqlshlib.resources"],
package_data={
"cqlshlib.resources": ["CQL.html", "CQL.css"],
},
include_package_data=True,
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

The 'include_package_data=True' setting is redundant when explicitly using 'package_data'. The 'include_package_data' option is used to include files specified in MANIFEST.in, but since you're explicitly listing the files in 'package_data', this setting is unnecessary. Consider removing it to avoid potential confusion or conflicts. If you want to keep it for other files, ensure you have a MANIFEST.in file or clarify the intent.

Suggested change
include_package_data=True,

Copilot uses AI. Check for mistakes.
ext_modules=get_extensions(),
)
Loading