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

Fix how exc.response is checked while sending POST request to API #85

Conversation

amandahla
Copy link
Collaborator

Overview

While refreshing the charm with Mjolnir enabled, the charm will try to register the Mjolnir user again. This was throwing a RegisterUserError instead of checking that the API was returning that the User ID was already taken and then proceeding with getting the access token.

Rationale

Fix how the response is checked in cases where the user already exists so the Mjolnir configuration can proceed.

Juju Events Changes

N/A

Module Changes

N/A

Library Changes

N/A

Checklist

@amandahla amandahla requested a review from a team as a code owner October 23, 2023 14:52
@amandahla amandahla changed the title Fix how exc.response is checked while send POST request to API Fix how exc.response is checked while sending POST request to API Oct 23, 2023
arturo-seijas
arturo-seijas previously approved these changes Oct 23, 2023
Copy link
Collaborator

@arturo-seijas arturo-seijas left a comment

Choose a reason for hiding this comment

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

LGTM

@amandahla
Copy link
Collaborator Author

This PR will fix #80

weiiwang01
weiiwang01 previously approved these changes Oct 24, 2023
Copy link
Contributor

@gregory-schiano gregory-schiano left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link
Contributor

Test coverage for 43be3a5

Name                            Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------------------------------
src/actions/__init__.py             3      0      0      0   100%
src/actions/register_user.py       22      0      2      0   100%
src/actions/reset_instance.py      21      0      2      0   100%
src/charm.py                      175      8     32      4    94%   124-125, 177-178, 197-198, 239, 253
src/charm_state.py                 63      1     12      1    97%   138
src/charm_types.py                 11      0      0      0   100%
src/database_client.py             53      1     10      3    94%   35, 47->exit, 69->exit
src/database_observer.py           54      4      6      0    93%   70-72, 88
src/exceptions.py                   4      0      0      0   100%
src/mjolnir.py                     76      8     20      2    88%   60-64, 73, 93-96
src/observability.py                9      0      0      0   100%
src/pebble.py                      75      9      4      2    86%   90-91, 93, 107-112
src/saml_observer.py               45      1      8      0    98%   64
src/synapse/__init__.py             3      0      0      0   100%
src/synapse/api.py                161      2     22      2    98%   207, 376
src/synapse/workload.py           192      6     30      5    95%   343-344, 360, 409->412, 458, 460, 465
src/user.py                        24      0      4      0   100%
---------------------------------------------------------------------------
TOTAL                             991     40    152     19    95%

Static code analysis report

Run started:2023-10-25 08:30:25.824231

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 4996
  Total lines skipped (#nosec): 4
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

@gregory-schiano gregory-schiano merged commit 0c6c8f9 into main Oct 25, 2023
20 checks passed
@gregory-schiano gregory-schiano deleted the ISD-1189-error-while-refreshing-charm-when-mjolnir-is-enabled branch October 25, 2023 11:55
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