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

Adds missing steps for installation on MacOS X #791

Merged
merged 4 commits into from May 10, 2019

Conversation

kevprice83
Copy link
Member

What this PR does / why we need it:

Some steps are missing in the installation guide for MacOS.

Which issue(s) this PR fixes

THREESCALE-2489

INSTALL.md Outdated
@@ -176,3 +184,16 @@ Start up the rails server by running the following command:
$ UNICORN_WORKERS=2 rails server -b 0.0.0.0 # Runs the server, available at localhost:3000
```
> The number of unicorn workers is variable and sometimes it will need more than 2. In case the server is slow or start suffering from timeouts, try restarting porta with a higher number like 8.

Set the domain for the Master, Provider & Developer portals via the rails console. Whichever hostname being used for the local machine should be the value set for each attribute (the following domain names are just examples and updating the */etc/hosts* file is also required):
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not necessary at all. It creates the domains by default. It is still ok to do it and mention it somewhere if you want, but this is not part of the installation so it shouldn't be in INSTALL.md, and anyway it is important to say clearly that this step is optional.

Copy link
Member Author

Choose a reason for hiding this comment

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

ah okay yes it should be optional, it creates the domains but they cannot be used, they are just example.com, is there another README where this step would be more appropriate?

Copy link
Contributor

Choose a reason for hiding this comment

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

@kevprice83 What do you mean that they can't be used? 😱 They should be able to be used. I can use them:

Tenant

image

Master

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Well the accounts were created in my case with the following domains: master-admin.example.com, provider-admin.example.com, provider.example.com so these were obviously never going to work unless they are added to the /etc/hosts. Did you pass a param when starting up the rails server to add .lvh.me? @Martouta

Copy link
Member

Choose a reason for hiding this comment

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

The .lvh.me resolves itself and all its subdomains to 127.0.0.1. So you don't need to setup your own DNS or change your /etc/hosts.

Copy link
Contributor

@Martouta Martouta May 8, 2019

Choose a reason for hiding this comment

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

Didier is right 😄 And anyway you can change the example.com anyway by an ENV THREESCALE_SUPERDOMAIN or making a config/settings.yml with this with the default you want:

superdomain: <%= superdomain = ENV.fetch('THREESCALE_SUPERDOMAIN', 'example.com') %>

And the .lvh.me is because we don't want to resolve to real URLs in development and we used to use .dev but we had problems with it for an update of Chrome so now we use this one, as you can customise as well if you want to (but only if you know well what you are doing):
dev_gtld: <%= ENV.fetch('DEV_GTLD', 'lvh.me') %>

Copy link
Member Author

Choose a reason for hiding this comment

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

Understood @Martouta @didierofrivia I'll make the change, I didn't know this.

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as you have internet or a DNS server setup correctly with .lvh.me pointing to 127.0.0.1

But I think that is out of topic for the README, we can do a Pro-tip
What would be good is to begin to have wiki page for those pro-tips instead of files.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I just wasn't aware that lvh.me was a standard DNS entry mapped back to 127.0.0.1 I think this could be fine in another wiki or README I suppose.

Run [NPM](https://www.npmjs.com/) to install all the required Node modules:

```bash
npm install
Copy link
Contributor

Choose a reason for hiding this comment

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

Great! Thanks 👍

Martouta
Martouta previously approved these changes May 8, 2019
Copy link
Contributor

@Martouta Martouta left a comment

Choose a reason for hiding this comment

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

I am not sure if that note at the end is ideal but I guess it won't hurt 😄
LGTM 👍

@Martouta Martouta requested a review from a team May 8, 2019 16:23
@Martouta
Copy link
Contributor

Martouta commented May 8, 2019

Preferably wait for one more review if it comes in few days 😝

INSTALL.md Outdated
@@ -176,3 +184,5 @@ Start up the rails server by running the following command:
$ UNICORN_WORKERS=2 rails server -b 0.0.0.0 # Runs the server, available at localhost:3000
```
> The number of unicorn workers is variable and sometimes it will need more than 2. In case the server is slow or start suffering from timeouts, try restarting porta with a higher number like 8.

**NOTE:** 3scale porta will resolve all subdomains locally through use of `.lvh.me` domain extension.
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably add an example.

When you seed the application with rake db:setup it will create the provider account and the master account, respectively accessible through, http://provider-admin.example.com.lvh.me:3000 and http://master-account.example.com.lvh.me:3000

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this note would be better right after the db:setup command, and mentioning also that password should wrote down at that moment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that's a good point @josemigallas as I did have to reset password having missed that output on the terminal. I can also add an example of the domains which are seeded @hallelujah

@kevprice83 kevprice83 changed the title adds missing steps for installation on MacOS X [WIP] adds missing steps for installation on MacOS X May 9, 2019
@kevprice83 kevprice83 changed the title [WIP] adds missing steps for installation on MacOS X WIP adds missing steps for installation on MacOS X May 9, 2019
@Martouta
Copy link
Contributor

Martouta commented May 9, 2019

Please also sign all the commits the next time you push 😄

@codecov
Copy link

codecov bot commented May 10, 2019

Codecov Report

Merging #791 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #791      +/-   ##
==========================================
- Coverage    92.9%   92.87%   -0.03%     
==========================================
  Files        2414     2397      -17     
  Lines       78251    78057     -194     
==========================================
- Hits        72696    72499     -197     
- Misses       5555     5558       +3
Impacted Files Coverage Δ
lib/tasks/multitenant/tenants.rake 53.84% <0%> (-23.08%) ⬇️
app/lib/features/account_deletion_config.rb 93.33% <0%> (-6.67%) ⬇️
app/representers/usage_limit_representer.rb 93.75% <0%> (-0.37%) ⬇️
app/models/user.rb 95.05% <0%> (-0.06%) ⬇️
test/unit/user_test.rb 98.83% <0%> (-0.02%) ⬇️
.../find_and_delete_accounts_scheduled_worker_test.rb 100% <0%> (ø) ⬆️
test/unit/signup/result_test.rb 100% <0%> (ø) ⬆️
config/initializers/features.rb 100% <0%> (ø) ⬆️
...rkers/find_and_delete_scheduled_accounts_worker.rb 100% <0%> (ø) ⬆️
app/lib/event_store/repository.rb 100% <0%> (ø) ⬆️
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05adee1...23d80ca. Read the comment docs.

@kevprice83 kevprice83 changed the title WIP adds missing steps for installation on MacOS X Adds missing steps for installation on MacOS X May 10, 2019
@kevprice83
Copy link
Member Author

@Martouta I signed my latest commit but not sure of how to sign all the existing commits, is that still necessary for this PR?

@didierofrivia
Copy link
Member

@kevprice83 all commits must be signed.

@kevprice83 kevprice83 force-pushed the update-install-guide-THREESCALE-2489 branch from 77a15de to ba691fd Compare May 10, 2019 12:05
@kevprice83 kevprice83 force-pushed the update-install-guide-THREESCALE-2489 branch from ba691fd to 23d80ca Compare May 10, 2019 12:27
@kevprice83
Copy link
Member Author

@didierofrivia all the commits are signed correctly now right?

Copy link
Contributor

@Martouta Martouta left a comment

Choose a reason for hiding this comment

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

👍 💪

@Martouta Martouta requested a review from a team May 10, 2019 12:43
@hallelujah hallelujah merged commit 002bf69 into master May 10, 2019
@hallelujah hallelujah deleted the update-install-guide-THREESCALE-2489 branch May 10, 2019 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants