Fixes #33320 - Refer to FQDN instead of "Foreman server" in SmartProx… #988
Conversation
wbclark
force-pushed
the
33320_always_refer_to_fqdn
branch
from
4021df8 to
c4c43d2
Compare
wbclark
force-pushed
the
33320_always_refer_to_fqdn
branch
from
c4c43d2 to
80c2891
Compare
wbclark
force-pushed
the
33320_always_refer_to_fqdn
branch
from
80c2891 to
f414972
Compare
wbclark
force-pushed
the
33320_always_refer_to_fqdn
branch
from
f414972 to
a567fee
Compare
|
Fixed unit tests. There are still some failures for acceptance tests on Debian and Ubuntu, but to me they look unrelated to this change. |
| explanations = { | ||
| '400' => 'Something is wrong with the data sent to Foreman server', | ||
| '400' => "Something is wrong with the data sent to #{fqdn}", |
There was a problem hiding this comment.
Should this then also be sent to Foreman at #{fqdn} ?
There was a problem hiding this comment.
I think that would be both more specific and more consistent.
I will make the change
There was a problem hiding this comment.
I'll use this consistently throughout, i.e. also changing to "Often this is caused by invalid Oauth credentials sent to Foreman at #{fqdn}" and "The requested resource was not found in Foreman at #{fqdn}" for the explanations for 401 and 404 codes
…y registration error messages
wbclark
force-pushed
the
33320_always_refer_to_fqdn
branch
from
a567fee to
be1c87d
Compare
| '400' => "Something is wrong with the data sent to Foreman at #{fqdn}", | ||
| '401' => "Often this is caused by invalid Oauth credentials sent to Foreman at #{fqdn}", | ||
| '404' => "The requested resource was not found in Foreman at #{fqdn}", | ||
| '500' => "Check /var/log/foreman/production.log on #{fqdn} for detailed information", |
There was a problem hiding this comment.
| '500' => "Check /var/log/foreman/production.log on #{fqdn} for detailed information", | |
| '500' => "Check Foreman logs at /var/log/foreman/production.log on #{fqdn} for detailed information", |
Considering making this change as well
There was a problem hiding this comment.
I would keep it short. IMHO the path tells you enough what's there.
There was a problem hiding this comment.
OK, I basically agree. While this would be more consistent with the overall pattern, it doesn't really inform the user of anything new and therefore doesn't justify the added length.
| '503' => 'The webserver was unable to reach the backend service. Is foreman.service running?', | ||
| '504' => 'The webserver timed out waiting for a response from the backend service. Is Foreman under unusually heavy load?' | ||
| '400' => "Something is wrong with the data sent to Foreman at #{fqdn}", | ||
| '401' => "Often this is caused by invalid Oauth credentials sent to Foreman at #{fqdn}", |
There was a problem hiding this comment.
I'm not sure this change really adds anything. At the CLI we have oauth parameters. Those are wrong and the user needs to correct those. Does this comment help them find that out?
There was a problem hiding this comment.
The change does clarify that the issue is the Oauth credentials that are being sent. So it could dispel the notion that the credentials are wrong somewhere on the Foreman side.
The intent is that the user will compare 1. what does Foreman expect to receive? vs. 2. What am I passing to the CLI?
…y registration error messages