Skip to content
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

apiserver: do not access state in NewServer - fixes-1504578 #3485

Conversation

rogpeppe
Copy link
Contributor

We also ensure that the listener is closed when NewServer fails and that we allow an agent login without requiring the environment configuration (that's also potentially problematic if the user database needs a migration but not something we can easily sort out now).

We also remove apiclientSuite.TestConnectWebsocketPrefersLocalhostIfPresent because the logic it was supposed be testing has long been removed and didn't actually test that logic in the first place.

Fixes https://bugs.launchpad.net/juju-core/+bug/1504578.
fixes-1504578
['fixes-1504578']

(Review request: http://reviews.vapour.ws/r/2878/)

@rogpeppe rogpeppe force-pushed the 060-apiserver-no-NewServer-state-access branch 2 times, most recently from 1b43885 to 20d26a6 Compare October 14, 2015 09:55
@rogpeppe
Copy link
Contributor Author

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Oct 14, 2015

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot
Copy link
Collaborator

jujubot commented Oct 14, 2015

Build failed: Does not match ['fixes-1504578']
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/5081

@rogpeppe
Copy link
Contributor Author

$$merge$$

@rogpeppe rogpeppe changed the title apiserver: do not access state in NewServer apiserver: do not access state in NewServer - fixes-1504578 Oct 14, 2015
@jujubot
Copy link
Collaborator

jujubot commented Oct 14, 2015

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot
Copy link
Collaborator

jujubot commented Oct 14, 2015

Build failed: Does not match ['fixes-1504578']
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/5083

@rogpeppe
Copy link
Contributor Author

fixes-1504578

@rogpeppe
Copy link
Contributor Author

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Oct 14, 2015

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot
Copy link
Collaborator

jujubot commented Oct 14, 2015

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/5084

@rogpeppe rogpeppe force-pushed the 060-apiserver-no-NewServer-state-access branch from 20d26a6 to 2def502 Compare October 14, 2015 11:50
@rogpeppe
Copy link
Contributor Author

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Oct 14, 2015

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

jujubot added a commit that referenced this pull request Oct 14, 2015
…te-access

apiserver: do not access state in NewServer - fixes-1504578

We also ensure that the listener is closed when NewServer fails and that we allow an agent login without requiring the environment configuration (that's also potentially problematic if the user database needs a migration but not something we can easily sort out now).

We also remove apiclientSuite.TestConnectWebsocketPrefersLocalhostIfPresent because the logic it was supposed be testing has long been removed and didn't actually test that logic in the first place.

Fixes https://bugs.launchpad.net/juju-core/+bug/1504578.
fixes-1504578
['fixes-1504578']

(Review request: http://reviews.vapour.ws/r/2878/)
@jujubot jujubot merged commit e048625 into juju:chicago-cubs Oct 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants