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 a custom http header: Accept-Type-Allow-Int64 #27901

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

PowderLi
Copy link
Contributor

@PowderLi PowderLi commented Oct 24, 2023

/kind bug
to #27856
to #27988

@sre-ci-robot sre-ci-robot added the size/L Denotes a PR that changes 100-499 lines. label Oct 24, 2023
@mergify
Copy link
Contributor

mergify bot commented Oct 24, 2023

@PowderLi Please associate the related issue to the body of your Pull Request. (eg. “issue: #”)

@mergify
Copy link
Contributor

mergify bot commented Oct 24, 2023

@PowderLi ut workflow job failed, comment rerun ut can trigger the job again.

@PowderLi PowderLi force-pushed the master-js-int64-drawback branch from f25bf1a to c8f2fff Compare October 25, 2023 02:12
@mergify
Copy link
Contributor

mergify bot commented Oct 25, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@mergify
Copy link
Contributor

mergify bot commented Oct 25, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@PowderLi PowderLi force-pushed the master-js-int64-drawback branch from c8f2fff to b526853 Compare October 25, 2023 08:38
@mergify
Copy link
Contributor

mergify bot commented Oct 25, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@PowderLi
Copy link
Contributor Author

/run-cpu-e2e

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Merging #27901 (7e7e323) into master (f93ad64) will increase coverage by 0.01%.
Report is 6 commits behind head on master.
The diff coverage is 76.19%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #27901      +/-   ##
==========================================
+ Coverage   81.79%   81.81%   +0.01%     
==========================================
  Files         824      820       -4     
  Lines      116574   117054     +480     
==========================================
+ Hits        95353    95764     +411     
- Misses      18061    18121      +60     
- Partials     3160     3169       +9     
Files Coverage Δ
internal/core/src/storage/ChunkCache.cpp 81.25% <100.00%> (-14.21%) ⬇️
internal/core/src/storage/ChunkCache.h 100.00% <ø> (ø)
internal/distributed/proxy/httpserver/utils.go 84.78% <100.00%> (+1.14%) ⬆️
internal/querynodev2/segments/segment.go 76.24% <100.00%> (+0.49%) ⬆️
pkg/util/paramtable/http_param.go 100.00% <100.00%> (ø)
internal/core/src/storage/MinioChunkManager.cpp 0.00% <0.00%> (ø)
internal/distributed/proxy/service.go 79.77% <62.50%> (-0.20%) ⬇️
...nternal/distributed/proxy/httpserver/handler_v1.go 91.70% <82.87%> (-0.35%) ⬇️
internal/core/src/segcore/SegmentSealedImpl.cpp 73.53% <26.08%> (-1.73%) ⬇️

... and 165 files with indirect coverage changes

@@ -218,6 +218,11 @@ func generateSearchResult() []map[string]interface{} {
FieldBookIntro: []float32{0.3, 0.33},
HTTPReturnDistance: float32(0.09),
}
if dataType == schemapb.DataType_String {
Copy link
Collaborator

Choose a reason for hiding this comment

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

cover schemapb.DataType_VarChar

Copy link
Collaborator

Choose a reason for hiding this comment

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

you can modify this in next pr

StrId: &schemapb.StringArray{
Data: []string{"1", "2", "3"},
}
case schemapb.DataType_String:
Copy link
Collaborator

Choose a reason for hiding this comment

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

cover schemapb.DataType_VarChar

Copy link
Collaborator

Choose a reason for hiding this comment

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

you can modify this in next pr

@@ -404,7 +405,7 @@ func (h *Handlers) query(c *gin.Context) {
if err != nil {
c.JSON(http.StatusOK, gin.H{HTTPReturnCode: merr.Code(err), HTTPReturnMessage: err.Error()})
} else {
outputData, err := buildQueryResp(int64(0), response.OutputFields, response.FieldsData, nil, nil)
outputData, err := buildQueryResp(int64(0), response.OutputFields, response.FieldsData, nil, nil, strings.ToLower(c.Request.Header.Get(HTTPHeaderAllowInt64)) != "false")
Copy link
Collaborator

Choose a reason for hiding this comment

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

how about strconv.ParseBool ?

if val == "" {
httpParams := &paramtable.Get().HTTPCfg
if !httpParams.AcceptTypeAllowInt64.GetAsBool() {
c.Request.Header.Set(httpserver.HTTPHeaderAllowInt64, "false")
Copy link
Collaborator

Choose a reason for hiding this comment

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

do not need to set "true" in case of httpParams.AceptTypeAllocInt64 is true ?

@PowderLi PowderLi force-pushed the master-js-int64-drawback branch 2 times, most recently from d6a2bd0 to 2fc0d34 Compare October 26, 2023 11:18
@mergify mergify bot added the ci-passed label Oct 26, 2023
@PowderLi PowderLi force-pushed the master-js-int64-drawback branch from 2fc0d34 to 3e083ec Compare October 27, 2023 07:10
@mergify mergify bot removed the ci-passed label Oct 27, 2023
@mergify
Copy link
Contributor

mergify bot commented Oct 27, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@PowderLi PowderLi force-pushed the master-js-int64-drawback branch from 3e083ec to 4996cc3 Compare October 27, 2023 10:03
@mergify
Copy link
Contributor

mergify bot commented Oct 27, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@PowderLi PowderLi force-pushed the master-js-int64-drawback branch from 4996cc3 to 4992ff8 Compare October 30, 2023 05:08
@sre-ci-robot sre-ci-robot added size/XL Denotes a PR that changes 500-999 lines. and removed size/L Denotes a PR that changes 100-499 lines. labels Oct 30, 2023
@mergify
Copy link
Contributor

mergify bot commented Oct 30, 2023

@PowderLi ut workflow job failed, comment rerun ut can trigger the job again.

@PowderLi PowderLi force-pushed the master-js-int64-drawback branch 2 times, most recently from 0d082c7 to 8d5d9a1 Compare October 30, 2023 09:02
@sre-ci-robot sre-ci-robot added size/XXL Denotes a PR that changes 1000+ lines. and removed size/XL Denotes a PR that changes 500-999 lines. labels Oct 30, 2023
@PowderLi PowderLi force-pushed the master-js-int64-drawback branch from 8d5d9a1 to bfcc17a Compare October 30, 2023 14:49
@PowderLi PowderLi force-pushed the master-js-int64-drawback branch from bfcc17a to 7e7e323 Compare October 31, 2023 01:37
@mergify
Copy link
Contributor

mergify bot commented Oct 31, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@PowderLi
Copy link
Contributor Author

/run-cpu-e2e

@mergify
Copy link
Contributor

mergify bot commented Oct 31, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@czs007
Copy link
Collaborator

czs007 commented Oct 31, 2023

/approve
/lgtm

@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: czs007, PowderLi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot merged commit 0c0f012 into milvus-io:master Nov 1, 2023
11 of 12 checks passed
@PowderLi PowderLi deleted the master-js-int64-drawback branch November 14, 2023 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved ci-passed dco-passed DCO check passed. lgtm size/XXL Denotes a PR that changes 1000+ lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants