Skip to content

Commit 8f08440

Browse files
msrathore-dbmsrathore-db
committed
Included coverage check in the integration workflow to avoid running e2e tests twice
2 parents b75f4d2 + 36d3ec4 commit 8f08440

24 files changed

+1851
-534
lines changed

‎.github/workflows/coverage-check.yml‎

Lines changed: 0 additions & 131 deletions
This file was deleted.

‎.github/workflows/integration.yml‎

Lines changed: 76 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ jobs:
2222
#----------------------------------------------
2323
- name: Check out repository
2424
uses: actions/checkout@v4
25+
with:
26+
fetch-depth: 0 # Needed for coverage comparison
27+
ref: ${{ github.event.pull_request.head.ref || github.ref_name }}
28+
repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }}
2529
- name: Set up python
2630
id: setup-python
2731
uses: actions/setup-python@v5
@@ -50,9 +54,78 @@ jobs:
5054
# install dependencies if cache does not exist
5155
#----------------------------------------------
5256
- name: Install dependencies
57+
if: steps.cached-poetry-dependencies.outputs.cache-hit != 'true'
58+
run: poetry install --no-interaction --no-root
59+
#----------------------------------------------
60+
# install your root project, if required
61+
#----------------------------------------------
62+
- name: Install library
5363
run: poetry install --no-interaction --all-extras
5464
#----------------------------------------------
55-
# run test suite
65+
# run all tests with coverage
66+
#----------------------------------------------
67+
- name: Run tests with coverage
68+
run: |
69+
poetry run python -m pytest \
70+
tests/unit tests/e2e \
71+
--cov=src --cov-report=xml --cov-report=term -v
72+
#----------------------------------------------
73+
# check for coverage override
74+
#----------------------------------------------
75+
- name: Check for coverage override
76+
id: override
77+
run: |
78+
OVERRIDE_COMMENT=$(echo "${{ github.event.pull_request.body }}" | grep -E "SKIP_COVERAGE_CHECK\s*=" || echo "")
79+
if [ -n "$OVERRIDE_COMMENT" ]; then
80+
echo "override=true" >> $GITHUB_OUTPUT
81+
REASON=$(echo "$OVERRIDE_COMMENT" | sed -E 's/.*SKIP_COVERAGE_CHECK\s*=\s*(.+)/\1/')
82+
echo "reason=$REASON" >> $GITHUB_OUTPUT
83+
echo "Coverage override found in PR description: $REASON"
84+
else
85+
echo "override=false" >> $GITHUB_OUTPUT
86+
echo "No coverage override found"
87+
fi
88+
#----------------------------------------------
89+
# check coverage percentage
90+
#----------------------------------------------
91+
- name: Check coverage percentage
92+
if: steps.override.outputs.override == 'false'
93+
run: |
94+
COVERAGE_FILE="coverage.xml"
95+
if [ ! -f "$COVERAGE_FILE" ]; then
96+
echo "ERROR: Coverage file not found at $COVERAGE_FILE"
97+
exit 1
98+
fi
99+
100+
# Install xmllint if not available
101+
if ! command -v xmllint &> /dev/null; then
102+
sudo apt-get update && sudo apt-get install -y libxml2-utils
103+
fi
104+
105+
COVERED=$(xmllint --xpath "string(//coverage/@lines-covered)" "$COVERAGE_FILE")
106+
TOTAL=$(xmllint --xpath "string(//coverage/@lines-valid)" "$COVERAGE_FILE")
107+
PERCENTAGE=$(python3 -c "covered=${COVERED}; total=${TOTAL}; print(round((covered/total)*100, 2))")
108+
109+
echo "Branch Coverage: $PERCENTAGE%"
110+
echo "Required Coverage: 85%"
111+
112+
# Use Python to compare the coverage with 85
113+
python3 -c "import sys; sys.exit(0 if float('$PERCENTAGE') >= 85 else 1)"
114+
if [ $? -eq 1 ]; then
115+
echo "ERROR: Coverage is $PERCENTAGE%, which is less than the required 85%"
116+
exit 1
117+
else
118+
echo "SUCCESS: Coverage is $PERCENTAGE%, which meets the required 85%"
119+
fi
120+
121+
#----------------------------------------------
122+
# coverage enforcement summary
56123
#----------------------------------------------
57-
- name: Run e2e tests
58-
run: poetry run python -m pytest tests/e2e
124+
- name: Coverage enforcement summary
125+
run: |
126+
if [ "${{ steps.override.outputs.override }}" == "true" ]; then
127+
echo "⚠️ Coverage checks bypassed: ${{ steps.override.outputs.reason }}"
128+
echo "Please ensure this override is justified and temporary"
129+
else
130+
echo "✅ Coverage checks enforced - minimum 85% required"
131+
fi

‎src/databricks/sql/auth/retry.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ def __init__(
127127
total=_attempts_remaining,
128128
respect_retry_after_header=True,
129129
backoff_factor=self.delay_min,
130-
allowed_methods=["POST"],
130+
allowed_methods=["POST", "GET", "DELETE"],
131131
status_forcelist=[429, 503, *self.force_dangerous_codes],
132132
)
133133

‎src/databricks/sql/backend/sea/backend.py‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
WaitTimeout,
2020
MetadataCommands,
2121
)
22+
from databricks.sql.backend.sea.utils.normalize import normalize_sea_type_to_thrift
2223
from databricks.sql.thrift_api.TCLIService import ttypes
2324

2425
if TYPE_CHECKING:
@@ -157,6 +158,7 @@ def __init__(
157158
)
158159

159160
self.use_hybrid_disposition = kwargs.get("use_hybrid_disposition", True)
161+
self.use_cloud_fetch = kwargs.get("use_cloud_fetch", True)
160162

161163
# Extract warehouse ID from http_path
162164
self.warehouse_id = self._extract_warehouse_id(http_path)
@@ -322,6 +324,11 @@ def _extract_description_from_manifest(
322324
# Format: (name, type_code, display_size, internal_size, precision, scale, null_ok)
323325
name = col_data.get("name", "")
324326
type_name = col_data.get("type_name", "")
327+
328+
# Normalize SEA type to Thrift conventions before any processing
329+
type_name = normalize_sea_type_to_thrift(type_name, col_data)
330+
331+
# Now strip _TYPE suffix and convert to lowercase
325332
type_name = (
326333
type_name[:-5] if type_name.endswith("_TYPE") else type_name
327334
).lower()
@@ -688,7 +695,7 @@ def get_catalogs(
688695
max_bytes=max_bytes,
689696
lz4_compression=False,
690697
cursor=cursor,
691-
use_cloud_fetch=False,
698+
use_cloud_fetch=self.use_cloud_fetch,
692699
parameters=[],
693700
async_op=False,
694701
enforce_embedded_schema_correctness=False,
@@ -721,7 +728,7 @@ def get_schemas(
721728
max_bytes=max_bytes,
722729
lz4_compression=False,
723730
cursor=cursor,
724-
use_cloud_fetch=False,
731+
use_cloud_fetch=self.use_cloud_fetch,
725732
parameters=[],
726733
async_op=False,
727734
enforce_embedded_schema_correctness=False,
@@ -762,7 +769,7 @@ def get_tables(
762769
max_bytes=max_bytes,
763770
lz4_compression=False,
764771
cursor=cursor,
765-
use_cloud_fetch=False,
772+
use_cloud_fetch=self.use_cloud_fetch,
766773
parameters=[],
767774
async_op=False,
768775
enforce_embedded_schema_correctness=False,
@@ -809,7 +816,7 @@ def get_columns(
809816
max_bytes=max_bytes,
810817
lz4_compression=False,
811818
cursor=cursor,
812-
use_cloud_fetch=False,
819+
use_cloud_fetch=self.use_cloud_fetch,
813820
parameters=[],
814821
async_op=False,
815822
enforce_embedded_schema_correctness=False,

‎src/databricks/sql/backend/sea/result_set.py‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,20 +92,19 @@ def _convert_json_types(self, row: List[str]) -> List[Any]:
9292
converted_row = []
9393

9494
for i, value in enumerate(row):
95+
column_name = self.description[i][0]
9596
column_type = self.description[i][1]
9697
precision = self.description[i][4]
9798
scale = self.description[i][5]
9899

99-
try:
100-
converted_value = SqlTypeConverter.convert_value(
101-
value, column_type, precision=precision, scale=scale
102-
)
103-
converted_row.append(converted_value)
104-
except Exception as e:
105-
logger.warning(
106-
f"Error converting value '{value}' to {column_type}: {e}"
107-
)
108-
converted_row.append(value)
100+
converted_value = SqlTypeConverter.convert_value(
101+
value,
102+
column_type,
103+
column_name=column_name,
104+
precision=precision,
105+
scale=scale,
106+
)
107+
converted_row.append(converted_value)
109108

110109
return converted_row
111110

0 commit comments

Comments
 (0)