Update _appengine_environ.py #1471
Conversation
Fixed Bug #1470
Codecov Report
@@ Coverage Diff @@
## master #1471 +/- ##
==========================================
- Coverage 100% 98.12% -1.88%
==========================================
Files 22 22
Lines 1824 1863 +39
==========================================
+ Hits 1824 1828 +4
- Misses 0 35 +35
Continue to review full report at Codecov.
|
|
An other possibility would be https://github.com/potatolondon/djangae/blob/master/djangae/environment.py#L50 |
|
The tests failures don't seem related to the changes, so maybe retrying will fix the failures? |
|
Who can restart the tests? |
|
Members/owners can do it. You can also do it by closing/reopening the pull request! |
|
Let's give it a try |
|
So we're getting some really strange behavior on both CIs. Travis is borking out on some IPv6 availability and AppVeyor is timing out after an hour of execution on each build. From this PR I can't see why either of those things are happening? Can you try rebasing your branch and pushing that? |
|
Maybe the issue is that is_prod_appengine is now returning True in situations where it returned False in the past? |
|
That's totally it, thanks for the sanity check @pquentin :) |
|
Maybe APPENGINE_RUNTIME is the solution. |
|
I'll wait for @theacodes for final review of this. Thanks! :) |
|
|
||
|
|
||
| def is_appengine_sandbox(): | ||
| return is_appengine() and not is_prod_appengine_mvms() | ||
|
|
||
|
|
||
| def is_local_appengine(): | ||
| return ('APPENGINE_RUNTIME' in os.environ and | ||
| 'Development/' in os.environ['SERVER_SOFTWARE']) | ||
| return is_appengine() and \ |
There was a problem hiding this comment.
This isn't going to work right. This will return True for Flex as SERVER_SOFTWARE won't be set but APPENGINE_RUNTIME will be. We either need to require SERVER_SOFTWARE as part of the local check or find a more reliable way to determine whether or not you're in the local environment.
There was a problem hiding this comment.
Why is_local_appengine needs to know weather it is an flex environment or not?
For this distinction we use is_prod_appengine_mvms.
There was a problem hiding this comment.
The alternative is to add not is_prod_appengine_mvms to is_prod_appengine and is_local_appengine
There was a problem hiding this comment.
Basically each of these checks needs to be definitive:
is_local_appengine should return True if and only if you're on the local app engine dev server.
is_prod_appenegine should return Tue if and only if you're in the production app engine (standard or flex).
and so on. Does that make sense?
There was a problem hiding this comment.
What about the import check?
Fixed Bug #1470