-
Notifications
You must be signed in to change notification settings - Fork 34
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
@W-11993356 Metecho: handle scratch org DNS propagation delays #2075
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a big fan of formatting changes being mixed into the same PR as logic changes.
Are you using a different version of black
than the one that was used the last time these files were touched?
Other changes inline.
metecho/api/sf_run_flow.py
Outdated
if total_wait_time >= settings.MAXIMUM_JOB_LENGTH: | ||
raise ScratchOrgError( | ||
f"Failed to build your scratch org after {settings.MAXIMUM_JOB_LENGTH} seconds." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if total_wait_time >= settings.MAXIMUM_JOB_LENGTH: | |
raise ScratchOrgError( | |
f"Failed to build your scratch org after {settings.MAXIMUM_JOB_LENGTH} seconds." | |
assert total_wait_time >= settings.MAXIMUM_JOB_LENGTH: | |
raise ScratchOrgError( | |
f"Failed to build your scratch org after {settings.MAXIMUM_JOB_LENGTH} seconds." |
requirements/dev.in
Outdated
@@ -22,5 +22,6 @@ pytest-factoryboy | |||
pytest-lazy-fixture | |||
pytest-mock | |||
pytest-sugar | |||
responses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realize that the project wasn't already using responses.
We should decide if we are actually planning to use responses more generally. Adding it to the project trigger a single exception is probably overkill. One line of mocking could achieve the same thing.
Adding the one line of mocking is probably clearer anyways.
Let's just find out what Requests code is calling the Responses code and throw an exception in it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could approve as-is but if we have time to make these changes then let's do it.
76a1a18
to
006859d
Compare
No description provided.