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

Add ShowSpaces for Session #315

Merged
merged 2 commits into from
Feb 27, 2024
Merged

Conversation

haoxins
Copy link
Collaborator

@haoxins haoxins commented Feb 4, 2024

The reason for this is that we often use ConnPool to init the space but not the SessionPool

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

Description:

How do you solve it?

Special notes for your reviewer, ex. impact of this fix, design document, etc:

@codecov-commenter
Copy link

codecov-commenter commented Feb 4, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 64.59%. Comparing base (5135309) to head (b869d1e).

Files Patch % Lines
session.go 47.36% 7 Missing and 3 partials ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #315      +/-   ##
==========================================
- Coverage   64.73%   64.59%   -0.14%     
==========================================
  Files          11       11              
  Lines        2705     2723      +18     
==========================================
+ Hits         1751     1759       +8     
- Misses        810      817       +7     
- Partials      144      147       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@haoxins haoxins marked this pull request as ready for review February 4, 2024 07:50
@haoxins
Copy link
Collaborator Author

haoxins commented Feb 4, 2024

BTW, there is no tests for the ConnPool ~

wey-gu
wey-gu previously approved these changes Feb 4, 2024
@wey-gu wey-gu requested a review from Nicole00 February 4, 2024 07:57
@Nicole00
Copy link
Contributor

Nicole00 commented Feb 4, 2024

ConnectionPool is not designed to support to run execute, the user entry is Session.
image

@haoxins
Copy link
Collaborator Author

haoxins commented Feb 4, 2024

@Nicole00

ConnectionPool is not designed to support to run execute, the user entry is Session.

So can we move the methods into Session?

@haoxins haoxins force-pushed the feat-ConnectionPool branch 4 times, most recently from 7dffe66 to 051c775 Compare February 4, 2024 08:34
@haoxins haoxins changed the title Add ShowSpaces and Execute for ConnectionPool Add ShowSpaces and Execute for Session Feb 4, 2024
@haoxins haoxins changed the title Add ShowSpaces and Execute for Session Add ShowSpaces for Session Feb 4, 2024
@haoxins haoxins force-pushed the feat-ConnectionPool branch from 051c775 to ceadc67 Compare February 4, 2024 08:39
@haoxins haoxins force-pushed the feat-ConnectionPool branch from ceadc67 to 692981f Compare February 19, 2024 02:38
@haoxins haoxins force-pushed the feat-ConnectionPool branch from 67a473d to e52e4a0 Compare February 21, 2024 12:05
@Nicole00
Copy link
Contributor

@Nicole00

ConnectionPool is not designed to support to run execute, the user entry is Session.

So can we move the methods into Session?

Sorry for reply late. I think it's ok to provide the method for session.

@Nicole00 Nicole00 merged commit 5bfccb7 into vesoft-inc:master Feb 27, 2024
23 checks passed
@haoxins haoxins deleted the feat-ConnectionPool branch February 27, 2024 08:11
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.

4 participants