Skip to content

feat: load jina auth token from environment#368

Merged
alaeddine-13 merged 5 commits into
mainfrom
feat-env-auth-token
May 30, 2022
Merged

feat: load jina auth token from environment#368
alaeddine-13 merged 5 commits into
mainfrom
feat-env-auth-token

Conversation

@gmastrapas

Copy link
Copy Markdown
Contributor

Goals:

  • Enable passing Jina authentication token from the environment for da.push and DocumentArray.pull. Uses key JINA_AUTH_TOKEN
  • check and update documentation, if required. See guide

@codecov

codecov Bot commented May 24, 2022

Copy link
Copy Markdown

Codecov Report

Merging #368 (37542d5) into main (e918f62) will increase coverage by 27.52%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##             main     #368       +/-   ##
===========================================
+ Coverage   56.98%   84.51%   +27.52%     
===========================================
  Files         134      134               
  Lines        6324     6368       +44     
===========================================
+ Hits         3604     5382     +1778     
+ Misses       2720      986     -1734     
Flag Coverage Δ
docarray 84.51% <100.00%> (+27.52%) ⬆️

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

Impacted Files Coverage Δ
docarray/array/mixins/io/pushpull.py 88.88% <100.00%> (+66.79%) ⬆️
docarray/helper.py 64.43% <0.00%> (+1.10%) ⬆️
docarray/base.py 97.29% <0.00%> (+1.35%) ⬆️
docarray/array/storage/memory/find.py 43.85% <0.00%> (+2.04%) ⬆️
docarray/document/mixins/helper.py 70.00% <0.00%> (+2.55%) ⬆️
docarray/score/data.py 92.85% <0.00%> (+3.57%) ⬆️
docarray/array/mixins/io/from_gen.py 83.63% <0.00%> (+3.63%) ⬆️
docarray/document/mixins/porting.py 93.15% <0.00%> (+5.47%) ⬆️
docarray/math/ndarray.py 90.00% <0.00%> (+6.00%) ⬆️
docarray/array/mixins/io/binary.py 97.45% <0.00%> (+7.00%) ⬆️
... and 67 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e918f62...37542d5. Read the comment docs.

_get_auth_token.cache_clear()

token = 'test-auth-token'
os.environ['JINA_AUTH_TOKEN'] = token

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.

this should be handled with a fixture to make sure this does not pollute other tests in case of Exceptions

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I unset the env var in the same tests

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.

yes, but what if there is an Exception in the middle, then this part of the code is not executed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok makes sense

Comment thread tests/unit/array/mixins/test_pushpull.py Outdated
Comment thread docarray/array/mixins/io/pushpull.py Outdated
@pytest.mark.parametrize(
'set_env_vars', [{'JINA_AUTH_TOKEN': 'test-auth-token'}], indirect=True
)
def test_api_authorization_header_from_env(mocker, monkeypatch, set_env_vars):

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 just add one last test that asserts the priority in case both are set

@github-actions github-actions Bot added size/m and removed size/s labels May 27, 2022
@alaeddine-13 alaeddine-13 merged commit 1b79312 into main May 30, 2022
@alaeddine-13 alaeddine-13 deleted the feat-env-auth-token branch May 30, 2022 07:43
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.

3 participants