-
Notifications
You must be signed in to change notification settings - Fork 2k
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
UI: Cross region server monitor #9913
Conversation
Ember Asset Size actionAs of 2c13731 Files that got Bigger 🚨:
Files that stayed the same size 🤷:
|
f7e979e
to
b2faf8a
Compare
Ember Test Audit comparison
|
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.
nice minimal intervention, there’s one tiny duplicated line but 👍🏻 otherwise 😀
@@ -72,13 +72,15 @@ module('Integration | Component | agent-monitor', function(hooks) { | |||
assert.ok(logRequest.url.startsWith('/v1/agent/monitor')); | |||
assert.ok(logRequest.url.includes('client_id=client1')); | |||
assert.ok(logRequest.url.includes('log_level=info')); | |||
assert.ok(logRequest.url.includes('log_level=info')); |
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.
Is this meant to be a different assertion? Or accidental copypaste? 😯
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 think this was vim betraying me? Dunno, but I'll clean it up.
The region will naturally be appended to URLs via token.authorizedRequest but agent members includes all servers across all regions so relying on the application-level region isn't good enough.
b2faf8a
to
2c13731
Compare
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 going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes #8885
This was just some oversight. Switching regions and monitoring a server worked, but since the serves page lists all servers across all regions, we need to special case this one.