Skip to content

Conversation

FredAns
Copy link
Contributor

@FredAns FredAns commented May 12, 2022

We need to check if the connection to a DB Server has been initiated before accessing it.

@codecov
Copy link

codecov bot commented May 12, 2022

Codecov Report

Merging #1119 (632bd8a) into main (beb9c22) will increase coverage by 3.35%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1119      +/-   ##
==========================================
+ Coverage   74.62%   77.97%   +3.35%     
==========================================
  Files          43       43              
  Lines        6734     6760      +26     
==========================================
+ Hits         5025     5271     +246     
+ Misses       1709     1489     -220     

@germa89
Copy link
Collaborator

germa89 commented May 12, 2022

I will review it tomorrow.

Btw, you could use a decorator to avoid having to write the same stuff many times.

@germa89 germa89 self-requested a review May 12, 2022 16:10
@FredAns
Copy link
Contributor Author

FredAns commented May 12, 2022

I will review it tomorrow.

Btw, you could use a decorator to avoid having to write the same stuff many times.

Thanks German. Don't hesitate to suggest changes in my code. I just want to add some checks before we try to access to the DB Client stub.

Copy link
Collaborator

@germa89 germa89 left a comment

Choose a reason for hiding this comment

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

LGTM.

I have created a decorator which you can attach to any method/property, and it will check if DB is alive in the DB object or element object.

@germa89
Copy link
Collaborator

germa89 commented May 17, 2022

@FredAns is this still a draft?? Or can we merge?

@akaszynski akaszynski changed the title Add Checks before accessing DB Add checks before accessing database May 27, 2022
@akaszynski akaszynski marked this pull request as ready for review May 27, 2022 22:28
@akaszynski
Copy link
Collaborator

@FredAns is this still a draft?? Or can we merge?

Calling this good. v21.2.1 failed, I've restarted that.

Note: v21.2.1 fails seems to have a higher failure rate relative to v21.1.1 and v22.1.0. Let's keep that in mind is we want to skip over it for testing. I feel that if we can test before and after v21.2.1, we can ensure backwards compatibility while eliminating a flaky test.

@akaszynski akaszynski enabled auto-merge (squash) May 27, 2022 22:35
@akaszynski akaszynski merged commit 2a3b04e into main May 27, 2022
@akaszynski akaszynski deleted the FredAns/FixesBD branch May 27, 2022 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants