Skip to content
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.

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

CI Fix #992

Merged
merged 10 commits into from
Feb 27, 2024
Merged

CI Fix #992

merged 10 commits into from
Feb 27, 2024

Conversation

neelasha23
Copy link

@neelasha23 neelasha23 commented Feb 14, 2024

Describe your changes

  1. Removed test test_db_driver_error_raised because the latest DuckDB release is handling these cases:
Screenshot 2024-02-22 at 12 20 26 PM
  1. modified the sqlglot parsing logic to check for Nonetype.

Opened issues for the pending fixes:
#994
#993

Issue number

Closes #991

Checklist before requesting a review


📚 Documentation preview 📚: https://jupysql--992.org.readthedocs.build/en/992/

@neelasha23 neelasha23 added the no-changelog This PR doesn't require a changelog entry label Feb 14, 2024
empty

tests

tests

duckdb version

sqlglot version

nox

nox
empty
matplotlib version
lint

nox

sqlglot version
sqlglot version

sqlglot

revert changes

revert nox changes
@neelasha23 neelasha23 marked this pull request as ready for review February 22, 2024 17:00
@neelasha23
Copy link
Author

Please review @edublancas

src/sql/util.py Show resolved Hide resolved
@@ -469,11 +469,13 @@ def test_bar_two_col(load_data_two_col, ip):


@_cleanup_cm()
@pytest.mark.xfail(reason="Failing intermittently with DuckDB v0.10.0")

Choose a reason for hiding this comment

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

if duckdb is the problem, it'd be better to test with <0.10.0 and open an issue

please open the issue and then document why 0.10.0 is failing

Copy link
Author

@neelasha23 neelasha23 Feb 24, 2024

Choose a reason for hiding this comment

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

Have tagged a couple of issues in the PR description, please check if these are ok

@edublancas
Copy link

@neelasha23
Copy link
Author

neelasha23 commented Feb 27, 2024

@neelasha23 the tests are still failing: https://github.com/ploomber/jupysql/actions/runs/8007801460/job/21872791573?pr=992

Yes it's the clickhouse tests only. The original issue was for integration-test-non-live which is fixed. But later all the tests had started failing due to package updates. I have fixed some and opened a couple of issues for the remaining in case we want to merge the ones that are fixed. Opened an issue for the clickhouse tests and tagged in the PR description : #993

Pending DuckDB fixes: #994

I'm still figuring out the issue @edublancas

@edublancas edublancas merged commit f04ca41 into master Feb 27, 2024
21 of 23 checks passed
@edublancas edublancas deleted the issue991 branch February 27, 2024 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog This PR doesn't require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

integration tests are failing
2 participants