Skip to content
This repository was archived by the owner on Dec 4, 2024. It is now read-only.

Fix URIs not displayed in the configuration of applications #821

Merged
merged 1 commit into from
Mar 5, 2018

Conversation

clems4ever
Copy link
Contributor

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.

@orlandohohmeier orlandohohmeier self-requested a review February 28, 2018 23:56
@@ -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) => {
Copy link
Contributor

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)
Copy link
Contributor

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.

@orlandohohmeier
Copy link
Contributor

@clems4ever thanks a lot for addressing this one. I'd adjust the fetch checks, but the rest looks good!

@clems4ever
Copy link
Contributor Author

@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.
@clems4ever
Copy link
Contributor Author

Actually it's done.

@orlandohohmeier
Copy link
Contributor

@clems4ever once again thanks a lot for addressing this one. 🙇

@orlandohohmeier orlandohohmeier merged commit f70425b into d2iq-archive:master Mar 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants