Skip to content

set cluster name from user after authorization#715

Merged
zubenkoivan merged 3 commits intomasterfrom
set_cluster_name_from_user
May 22, 2019
Merged

set cluster name from user after authorization#715
zubenkoivan merged 3 commits intomasterfrom
set_cluster_name_from_user

Conversation

@zubenkoivan
Copy link
Copy Markdown
Contributor

@zubenkoivan zubenkoivan commented May 21, 2019

resolves #712

await jobs_service.create_job(mock_job_request, user)

@pytest.mark.asyncio
async def test_create_job_with_non_default_user_cluster_name(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please note that this test does not touch changes in authorized_user(): to test them, we need to add an integration test on api level

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.

@zubenkoivan zubenkoivan force-pushed the set_cluster_name_from_user branch from 830bc53 to a5439d4 Compare May 21, 2019 18:28
@zubenkoivan zubenkoivan requested a review from atemate May 21, 2019 18:30
Copy link
Copy Markdown
Collaborator

@dalazx dalazx left a comment

Choose a reason for hiding this comment

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

perfect. on green!

@codecov
Copy link
Copy Markdown

codecov bot commented May 22, 2019

Codecov Report

Merging #715 into master will increase coverage by 0.2%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #715     +/-   ##
=========================================
+ Coverage   93.92%   94.12%   +0.2%     
=========================================
  Files          32       32             
  Lines        3948     3948             
  Branches      422      422             
=========================================
+ Hits         3708     3716      +8     
+ Misses        179      172      -7     
+ Partials       61       60      -1
Impacted Files Coverage Δ
platform_api/user.py 94.28% <100%> (ø) ⬆️
platform_api/orchestrator/jobs_service.py 97.54% <0%> (+1.22%) ⬆️
platform_api/api.py 92.7% <0%> (+4.37%) ⬆️

@zubenkoivan zubenkoivan merged commit a3e94b4 into master May 22, 2019
@zubenkoivan zubenkoivan deleted the set_cluster_name_from_user branch May 22, 2019 14:32
@serhiy-storchaka
Copy link
Copy Markdown
Contributor

I think that HTTPInternalServerError leaked to the user is a bug. ClusterNotFound should be caught and converted to some other error code in the range 4xx.

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.

Integrate neuro_auth_client.User.cluser_name

5 participants