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.

Sign up for GitHub

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

Jump to bottom

Use OS hostname if hostname is undefined in settings #1309

Merged
merged 12 commits into from
Jun 2, 2015
Merged

Use OS hostname if hostname is undefined in settings #1309

merged 12 commits into from
Jun 2, 2015

Conversation

eugenk
Copy link
Member

@eugenk eugenk commented Apr 8, 2015

This automatically reads the hostname from the operating system and uses it in the application if hostname is not set in the settings.yml (as @jelmd requested).

@eugenk eugenk force-pushed the use_os_hostname_if_hostname_is_undefined_in_settings branch from d838f72 to 1abbb9a Compare April 19, 2015 08:06
@eugenk
Copy link
Member Author

eugenk commented Apr 19, 2015

I just rebased this on staging.

@eugenk eugenk mentioned this pull request Apr 19, 2015
Closed
@eugenk eugenk force-pushed the use_os_hostname_if_hostname_is_undefined_in_settings branch from 1abbb9a to db0cd50 Compare April 29, 2015 09:40
@eugenk
Copy link
Member Author

eugenk commented Apr 29, 2015

I rebased this on staging. Tests on travis and jenkins are going to be red because of #1327.

@eugenk eugenk force-pushed the use_os_hostname_if_hostname_is_undefined_in_settings branch from db0cd50 to 8232e28 Compare May 5, 2015 10:19
@eugenk eugenk force-pushed the use_os_hostname_if_hostname_is_undefined_in_settings branch from 8232e28 to 450d94e Compare May 19, 2015 12:46
0robustus1
0robustus1 reviewed May 20, 2015
View reviewed changes
config/initializers/hostname.rb
hostname
else
Addrinfo.tcp(tmp, 0).getnameinfo.first
Addrinfo.tcp(Socket.gethostname, 0).getnameinfo.first
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this fail with a more useful error message than SocketError: getaddrinfo: nodename nor servname provided, or not known?

@0robustus1
Copy link
Contributor

Ok let's start with the easy thing:
Unless a correct hostname/port-combo is placed inside the settings.yml, this approach won't work for development machines (it will default to localhost without a port, and development-machines don't run ontohub on port 80).

In addition to that, everywhere where you've used the url-rails helpers, and therefore set host: ..., you'll also need to set port: .... And for the (qualified) loc/ids i propose another helper method, which actually builds the complete authority (scheme + fqdn + port) in order to use it there, e.g. in lib/hets.rb.

@eugenk
Set fully qualified domain name config properly.
e2586f5
@eugenk
Use config.fqdn instead of Settings.hostname
820a918 
If `Settings.hostname` is defined, it is the same as `config.fqdn`,
so this is only really a change if `Settings.hostname` is `nil`.

This doesn't change the deployment config. There we explicitly use
`Settings.hostname` because we don't want to read the hostname from our
development machines when deploying.
@eugenk
Add port to url helper calls.
4c1d51e
@eugenk
Add and use url_autority method.
cb1256d
@eugenk
Copy link
Member Author

eugenk commented Jun 1, 2015

Unless a correct hostname/port-combo is placed inside the settings.yml, this approach won't work for development machines (it will default to localhost without a port, and development-machines don't run ontohub on port 80).

I don't understand this. The settings/development.yml specifies the port 3000.

@eugenk eugenk force-pushed the use_os_hostname_if_hostname_is_undefined_in_settings branch from 450d94e to b941d9b Compare June 1, 2015 13:53
@eugenk
Copy link
Member Author

eugenk commented Jun 1, 2015

Maybe the port problem lied in what b941d9b fixed. That commit also should have fixed #1345.

@eugenk
Copy link
Member Author

eugenk commented Jun 1, 2015

Now this should be done.

0robustus1
0robustus1 reviewed Jun 1, 2015
View reviewed changes
app/serializers/logic_mapping_serializer.rb
@@ -3,7 +3,9 @@ class Reference < ApplicationSerializer
attributes :iri

def iri
urls.logic_mapping_url(object, host: Settings.hostname)
urls.logic_mapping_url(object,
host: Settings.hostname,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why Settings.hostname instead of Ontohub::Application.config.fqdn?

@eugenk
Copy link
Member Author

eugenk commented Jun 2, 2015

Good catches. Looks like I forgot how to grep.

@0robustus1
Copy link
Contributor

Ran seeds and tests, seems ok now: 👍

eugenk added a commit that referenced this pull request Jun 2, 2015
@eugenk
Merge pull request #1309 from ontohub/use_os_hostname_if_hostname_is_…
e3a09e0 
…undefined_in_settings

Use OS hostname if hostname is undefined in settings
@eugenk eugenk merged commit e3a09e0 into staging Jun 2, 2015
@eugenk eugenk deleted the use_os_hostname_if_hostname_is_undefined_in_settings branch June 2, 2015 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ebolloff ebolloff

Labels
:pr: requested changes pending which is urgent
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@eugenk @0robustus1 @ebolloff