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

Migration to async DB drivers #60

Merged
merged 55 commits into from
Feb 13, 2023
Merged

Conversation

pseusys
Copy link
Collaborator

@pseusys pseusys commented Dec 27, 2022

Description

DB drivers changed to their async alternatives

Checklist

  • I have covered the code with tests
  • I have added comments to my code to help others understand it
  • I have updated the documentation to reflect the changes
  • I have performed a self-review of the changes

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Please provide a description: what is changed and why.

@pseusys pseusys requested review from kudep and RLKRo December 31, 2022 14:20
@pseusys pseusys self-assigned this Dec 31, 2022
dff/context_storages/__init__.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@kudep kudep left a comment

Choose a reason for hiding this comment

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

  • fix warnings
    image
  • fix test coverage
  • do same typing hints in all changed files like in this commit

dff/context_storages/database.py Outdated Show resolved Hide resolved
@pseusys
Copy link
Collaborator Author

pseusys commented Jan 15, 2023

For some reason, YDB test keps getting skipped.

@pseusys
Copy link
Collaborator Author

pseusys commented Feb 9, 2023

The only warning left is related to this bug, here it is said to be fixed until the end of month.

dff/context_storages/database.py Outdated Show resolved Hide resolved
tests/context_storages/test_dbs.py Outdated Show resolved Hide resolved
@pseusys pseusys requested review from kudep and RLKRo February 10, 2023 21:00
.coveragerc Show resolved Hide resolved
@pseusys
Copy link
Collaborator Author

pseusys commented Feb 13, 2023

@kudep, 97 - that's a lot. If we add just a few undocumented code lines (and sometimes that's inevitable) - and we'll be struggling to meet this high standard again. Maybe we should give ourselves some space for improvement and leave it at 96 or even 95?

@kudep
Copy link
Collaborator

kudep commented Feb 13, 2023

So, it's is not true, i mean, we have without excluding 93%, you exclude parts of code and now we have same real coverage with another threshold 97%.
Difference between in terms of lines of code same for 92-93% and 96-97%. It's mean that your question is not relevant If we add just a few undocumented code lines (and sometimes that's inevitable)...

#60 (comment)

@kudep
Copy link
Collaborator

kudep commented Feb 13, 2023

Also now we can see another issue: local coverage and GH Action's coverage is not match
97 vs 93
do you know about that?

Copy link
Collaborator

@kudep kudep left a comment

Choose a reason for hiding this comment

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

  • local and GH Action's covers are not same

@pseusys
Copy link
Collaborator Author

pseusys commented Feb 13, 2023

Defore you ask about warning, it's a bug in nest_asyncio.

setup.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
Co-authored-by: Roman Zlobin <trlkrot@gmail.com>
@kudep kudep merged commit 2c23486 into dev Feb 13, 2023
@kudep kudep deleted the refactor/async_db_drivers_migration branch February 13, 2023 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants