Skip to content
This repository was archived by the owner on Mar 24, 2026. It is now read-only.

Use /etc/hosts aliases instead of $DBHOST and other#2475

Merged
msmith-techempower merged 3 commits intoTechEmpower:masterfrom
NateBrady23:hosts
Jan 6, 2017
Merged

Use /etc/hosts aliases instead of $DBHOST and other#2475
msmith-techempower merged 3 commits intoTechEmpower:masterfrom
NateBrady23:hosts

Conversation

@NateBrady23
Copy link
Copy Markdown
Member

I noticed that TFB-client TFB-database and TFB-server were already being set up in the /etc/hosts file during vagrant bootstrapping. I've updated the travis setup to do the same. Before something like this gets merged in, a note in the Installation Guide for those not using vagrant is needed.

Ideally, we would no longer need to use $DBHOST and other environment variables for IP addresses. We would also no longer need to use sed in setup files to replace IPs. I've edited a few frameworks here as a proof of concept, but also, the suite uses benchmark.cfg.example if a benchmark.cfg doesn't exist. So for the sake of this travis run, $DBHOST=TFB-database.

mode=benchmark
os=linux
server_host=127.0.0.1
server_host=TFB-server
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There was once, long ago, an implementation similar to this; we removed it because some frameworks could not resolve names for reasons we could not explain. Hopefully, this is no longer the case....

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If it is the case, those frameworks can still do the sed stuff they're doing now and pull in the ip from the /etc/hosts file.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see this being done in travis and vagrant, but I don't see this setup happening in the general case (like our production code): you should plan on adding that in.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This would be part of the setup. So it would be something done manually (see the linked installation guide) or did you have something else in mind?

@NateBrady23
Copy link
Copy Markdown
Member Author

@msmith-techempower Here is the corresponding documentation: https://github.com/TechEmpower/TFB-Documentation/pull/58/files

All of the modified tests pass. We can leave $DBHOST and other environment variables available for now but push future tests and updates toward TFB-database, TFB-client, TFB-server.

If this is good for you, we can merge this in and I can update a lot of the others.

BUILD_DIR="${H2O_APP_HOME}_build"
H2O_APP_PROFILE_PORT="54321"
H2O_APP_PROFILE_URL="http://127.0.0.1:$H2O_APP_PROFILE_PORT"
H2O_APP_PROFILE_URL="http://TFB-database:$H2O_APP_PROFILE_PORT"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is wrong: The URL has nothing to do with the database (note that it is used as a parameter for curl); it is just the location at which the HTTP server is temporarily accessible while gathering the profiling data.

@NateBrady23
Copy link
Copy Markdown
Member Author

@volyrique Thanks. Going through some of the setups, to test this change. At a quick glance I misread the port as 5432 and associated it with postgres.

@msmith-techempower msmith-techempower merged commit c094138 into TechEmpower:master Jan 6, 2017
@NateBrady23 NateBrady23 deleted the hosts branch May 23, 2017 20:27
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.

3 participants