This repository was archived by the owner on Dec 4, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 74
Fix URIs not displayed in the configuration of applications #821
Merged
orlandohohmeier
merged 1 commit into
d2iq-archive:master
from
criteo-forks:fix-uris-v1.5.6
Mar 5, 2018
Merged
Fix URIs not displayed in the configuration of applications #821
orlandohohmeier
merged 1 commit into
d2iq-archive:master
from
criteo-forks:fix-uris-v1.5.6
Mar 5, 2018
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
orlandohohmeier
suggested changes
Mar 1, 2018
@@ -224,9 +223,12 @@ var AppVersionComponent = React.createClass({ | |||
? <UnspecifiedNodeComponent /> | |||
: getHighlightNode(appVersion.networks); | |||
|
|||
var urisNode = (appVersion.uris == null || appVersion.uris.length === 0) | |||
const uris = appVersion.fetch.map((fetch) => { |
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.
Let's add a check to verify that fetch
is defined and an array.
const uris = appVersion.fetch.map((fetch) => { | ||
return fetch["uri"]; | ||
}); | ||
var urisNode = (uris == null || uris.length === 0) |
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.
We can remove the uris == null
check as it will always be defined if fetch is defined.
@clems4ever thanks a lot for addressing this one. I'd adjust the |
@orlandohohmeier , I'll make the fix tomorrow. Thank you for having a look! |
After upgrading from Marathon 1.4.3 to 1.5.6, the display of URIs in the configuration view was set to "Unspecified" even if some URIs were set for the application. The problem was due to a change in the schema of /v2/apps. The "uris" field disappeared and has been completely replaced by the "fetch" field. This commit add tests covering an app declaring one and several URIs to detect the issue and fixes it. Warning: This fix might not be backward compatible with versions below 1.4.3.
0053b6c
to
7dd58fa
Compare
Actually it's done. |
orlandohohmeier
approved these changes
Mar 5, 2018
@clems4ever once again thanks a lot for addressing this one. 🙇 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After upgrading from Marathon 1.4.3 to 1.5.6, the display of URIs in the
configuration view was set to "Unspecified" even if some URIs were set
for the application. The problem was due to a change in the schema of
/v2/apps. The "uris" field disappeared and has been completely replaced
by the "fetch" field.
This commit add tests covering an app declaring one and several URIs to
detect the issue and fixes it.
Warning: This fix might not be backward compatible with versions below 1.4.3.