Skip to content

chore: split tests with older proto version#567

Closed
JoanFM wants to merge 21 commits into
mainfrom
feat-upgrade-qdrant-client
Closed

chore: split tests with older proto version#567
JoanFM wants to merge 21 commits into
mainfrom
feat-upgrade-qdrant-client

Conversation

@JoanFM

@JoanFM JoanFM commented Sep 27, 2022

Copy link
Copy Markdown
Member

Goals:
Refactor the tests so that tensorflow and paddle can work with a specific protobuf version

@codecov

codecov Bot commented Sep 27, 2022

Copy link
Copy Markdown

Codecov Report

Merging #567 (1731fc0) into main (c38d82d) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #567      +/-   ##
==========================================
- Coverage   88.28%   88.23%   -0.06%     
==========================================
  Files         134      133       -1     
  Lines        6640     6585      -55     
==========================================
- Hits         5862     5810      -52     
+ Misses        778      775       -3     
Flag Coverage Δ
docarray 88.23% <100.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
docarray/array/qdrant.py 100.00% <ø> (ø)
docarray/array/storage/annlite/backend.py 97.33% <100.00%> (+0.07%) ⬆️
docarray/array/storage/qdrant/backend.py 96.11% <100.00%> (+0.07%) ⬆️
docarray/array/storage/sqlite/backend.py 94.87% <100.00%> (+0.13%) ⬆️
docarray/array/storage/weaviate/backend.py 89.65% <100.00%> (+0.14%) ⬆️
docarray/math/distance/__init__.py

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@JoanFM JoanFM force-pushed the feat-upgrade-qdrant-client branch from aa1b795 to f7f623b Compare September 27, 2022 06:57
@JoanFM JoanFM linked an issue Sep 27, 2022 that may be closed by this pull request
1 task
@florian-hoenicke

Copy link
Copy Markdown

Any news regarding this? It would be very helpful for team-now.

@JoanFM

JoanFM commented Oct 4, 2022

Copy link
Copy Markdown
Member Author

Any news regarding this? It would be very helpful for team-now.

We are looking at some issues to fix the tests because of some protobuf dependency problem between qdrant and paddle paddle

@github-actions github-actions Bot added size/m and removed size/xs labels Oct 4, 2022
@JoanFM JoanFM force-pushed the feat-upgrade-qdrant-client branch from ca8bce3 to 56c8296 Compare October 4, 2022 15:54
@JoanFM JoanFM force-pushed the feat-upgrade-qdrant-client branch 2 times, most recently from 2fa10c7 to 0ee4d5e Compare October 5, 2022 07:51
@JoanFM JoanFM changed the title feat: update qdrant-client to v0.10.2 feat: update qdrant to v0.10.1 Oct 5, 2022
@JoanFM JoanFM closed this Oct 5, 2022
@JoanFM JoanFM reopened this Oct 5, 2022
@github-actions

github-actions Bot commented Oct 5, 2022

Copy link
Copy Markdown

📝 Docs are deployed on https://ft-feat-upgrade-qdrant-client--jina-docs.netlify.app 🎉

@JoanFM JoanFM force-pushed the feat-upgrade-qdrant-client branch from 33b8d92 to 2a3b74d Compare October 5, 2022 15:25
@JoanFM JoanFM closed this Oct 5, 2022
@JoanFM JoanFM reopened this Oct 5, 2022
@JoanFM JoanFM closed this Oct 5, 2022
@JoanFM JoanFM reopened this Oct 5, 2022
@JoanFM JoanFM closed this Oct 5, 2022
@JoanFM JoanFM reopened this Oct 5, 2022
@alaeddine-13

Copy link
Copy Markdown
Member

I would prefer entirely moving tests (even maybe test files) that contain tf/paddlepaddle, rather than copying test code

@github-actions github-actions Bot added size/m and removed size/l labels Oct 6, 2022
@JoanFM JoanFM requested a review from alaeddine-13 October 6, 2022 08:40
Comment thread .github/workflows/cd.yml
fail_ci_if_error: false
token: ${{ secrets.CODECOV_TOKEN }} # not required for public repos

docarray-oldproto-test:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

let's always reference old proto to protobuf 3 and new proto is protobuf 4

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I would let the pip dependency solution do the job, I do not want to say it here

Comment on lines -6 to -8
#declare -a array1=( "tests/unit/test_*.py" )
#declare -a array2=( $(ls -d tests/unit/*/ | grep -v '__pycache__' | grep -v 'array') )
#declare -a array3=( "tests/unit/array/*.py" )

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine to keep placeholder code for future

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I prefer to clear

@JoanFM JoanFM force-pushed the feat-upgrade-qdrant-client branch from dc82e06 to 1fa4772 Compare October 7, 2022 08:53
@JoanFM JoanFM force-pushed the feat-upgrade-qdrant-client branch from 1fa4772 to e91a9ee Compare October 7, 2022 08:53
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Base: 88.29% // Head: 88.01% // Decreases project coverage by -0.27% ⚠️

Coverage data is based on head (07f9982) compared to base (f4b16ef).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #567      +/-   ##
==========================================
- Coverage   88.29%   88.01%   -0.28%     
==========================================
  Files         134      133       -1     
  Lines        6648     6585      -63     
==========================================
- Hits         5870     5796      -74     
- Misses        778      789      +11     
Flag Coverage Δ
docarray 88.01% <ø> (-0.28%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
docarray/array/qdrant.py 100.00% <ø> (ø)
docarray/array/mixins/content.py 84.61% <0.00%> (-13.85%) ⬇️
docarray/array/storage/elastic/getsetdel.py 98.30% <0.00%> (-1.70%) ⬇️
docarray/array/storage/redis/getsetdel.py 95.45% <0.00%> (-1.52%) ⬇️
docarray/array/mixins/plot.py 67.09% <0.00%> (-0.87%) ⬇️
docarray/array/mixins/setitem.py 81.66% <0.00%> (-0.84%) ⬇️
docarray/math/distance/__init__.py

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@JoanFM

JoanFM commented Nov 7, 2022

Copy link
Copy Markdown
Member Author

Please @AnneYang720 , @alaeddine-13 reopen PR on your own

@JoanFM JoanFM closed this Nov 7, 2022
@JoanFM JoanFM deleted the feat-upgrade-qdrant-client branch November 7, 2022 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants