denisenkom / pytds

Python DBAPI driver for MSSQL using pure Python TDS (Tabular Data Stream) protocol implementation
MIT License
190 stars 52 forks source link

Enable readonly #138

Closed takoau closed 1 year ago

takoau commented 2 years ago
  1. Add type flags in TDS packet
  2. Correct type flag in tds.py
codecov[bot] commented 2 years ago

Codecov Report

Merging #138 (6d93320) into master (7c73812) will decrease coverage by 1.83%. The diff coverage is 69.23%.

:exclamation: Current head 6d93320 differs from pull request most recent head cd828c4. Consider uploading reports for the commit cd828c4 to get more accurate results

@@            Coverage Diff             @@
##           master     #138      +/-   ##
==========================================
- Coverage   93.58%   91.74%   -1.84%     
==========================================
  Files          28       27       -1     
  Lines        7947     7744     -203     
==========================================
- Hits         7437     7105     -332     
- Misses        510      639     +129     
Impacted Files Coverage Δ
src/pytds/__init__.py 90.99% <0.00%> (-1.44%) :arrow_down:
src/pytds/tds.py 95.90% <100.00%> (-0.63%) :arrow_down:
src/pytds/tds_base.py 93.71% <100.00%> (-1.41%) :arrow_down:
tests/connected_test.py 97.54% <100.00%> (-0.62%) :arrow_down:
src/pytds/tls.py 18.64% <0.00%> (-74.58%) :arrow_down:
tests/fixtures.py 93.33% <0.00%> (-6.67%) :arrow_down:
tests/unit_test.py 94.10% <0.00%> (-4.98%) :arrow_down:
tests/utils.py 81.13% <0.00%> (-1.89%) :arrow_down:
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7c73812...cd828c4. Read the comment docs.

takoau commented 2 years ago

Confirmed that this works for our usecase. Hi Ecly,

I am not a developer but a DBA so I'm not familiar with coding lifecycles. Could you tell how I can fix the codecov errors?

takoau commented 2 years ago

Confirmed that this works for our usecase. Hi Ecly,

I am not a developer but a DBA so I'm not familiar with coding lifecycles. Could you tell how I can fix the codecov errors?

@ecly

jhoareau commented 2 years ago

Hi @takoau the coverage decreasing might not be an issue (that will have to be taken up with @denisenkom), but some tests are failing in the CI, in particular the test_bulk_insert raises a timeout in some environments: https://ci.appveyor.com/project/denisenkom/pytds/builds/44202650/job/q08bnxgc7qg8tuui https://ci.appveyor.com/project/denisenkom/pytds/builds/44202650/job/ocybqh8ujgwtkxwg https://ci.appveyor.com/project/denisenkom/pytds/builds/44202650/job/gi8je55506nuohhg

However, it looks like the main author of this project has not accepted any PRs for the past year, so I am a bit worried about the future of this project. We have actually released a PyPi wheel based on your branch for our needs: https://pypi.org/project/python-tds-readonly-fork/1.11.1/

takoau commented 2 years ago

Hi @takoau the coverage decreasing might not be an issue (that will have to be taken up with @denisenkom), but some tests are failing in the CI, in particular the test_bulk_insert raises a timeout in some environments: https://ci.appveyor.com/project/denisenkom/pytds/builds/44202650/job/q08bnxgc7qg8tuui https://ci.appveyor.com/project/denisenkom/pytds/builds/44202650/job/ocybqh8ujgwtkxwg https://ci.appveyor.com/project/denisenkom/pytds/builds/44202650/job/gi8je55506nuohhg

However, it looks like the main author of this project has not accepted any PRs for the past year, so I am a bit worried about the future of this project. We have actually released a PyPi wheel based on your branch for our needs: https://pypi.org/project/python-tds-readonly-fork/1.11.1/

Hey @jhoareau thanks for your explanation and the appreciation on the wheel release. To be honest I have no idea with coverage before you told me. :) On the other hand, I suspect the test case has some issues that, all pending PRs have the same failed cases on bulk insert timeout. PR#138 PR#130

This test case went well if a new table is created. I reverted this change yesterday as it's better not to mess up the original test cases.

cur.execute('create table myschema.bulk_insert_table(num int not null, data varchar(100))')
cur.copy_to(f, 'bulk_insert_table', schema='myschema', columns=('num', 'data'))
cur.execute('select num, data from myschema.bulk_insert_table')
assert [(42, 'foo'), (74, 'bar')] == cur.fetchall()