Closed Bug 951709 Opened 11 years ago Closed 10 years ago

Change ENV variables called "Hostname" to something that doesn't confuse Linux

Categories

(Webmaker Graveyard :: General, defect)

Product:
Webmaker Graveyard
Component:
General
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: sedge, Assigned: admix)

Details

Attachments

(6 files, 3 obsolete files)

https://github.com/mozilla/goggles.webmaker.org/pull/120
56 bytes, text/x-github-pull-request
cade
: review+
Details | Review
https://github.com/mozilla/popcorn.webmaker.org/pull/522
56 bytes, text/x-github-pull-request
cade
: review+
Details | Review
https://github.com/mozilla/thimble.webmaker.org/pull/401
56 bytes, text/x-github-pull-request
cade
: review+
Details | Review
https://github.com/mozilla/webmaker.org/pull/643
48 bytes, text/x-github-pull-request
cade
: review+
jon
: feedback+
Details | Review
https://github.com/mozilla/login.webmaker.org/pull/251
54 bytes, text/x-github-pull-request
cade
: review+
Details | Review
https://github.com/mozilla/popcorn.webmaker.org/pull/526
56 bytes, text/x-github-pull-request
cade
: review+
Details | Review 
Apparently our use of the ENV var `hostname` is conflicting with a system variable called $HOSTNAME on some linux systems, causing some bizzare bugs.

Flagging :cade for more info.
Flags: needinfo?(cade)
Not sure what you want in terms of info, your explanation was accurate. 

To re-iterate Linux systems (At least Fedora and Ubuntu) define an environment variable "HOSTNAME" which is by default set to something like "localhost.localdomain". Our apps have a similar variable that defines the hostname the app lives at and it is used for various purposes, such as rendering HTML with correct URLs.

The problem arises when we run the node app. This HOSTNAME variable is defined in a users terminal session, normally via .bashrc or something similar. It is passed into the app automatically, and Habitat - our configuration variable system - will prefer that value over any values defined in a .env file. This produces bad links, for example, with the new user form.
Flags: needinfo?(cade)
Yeah, makes sense to change it then. Before landing, can you flag me as a needinfo? so that I can update the server configs?
Assignee: nobody → kieran.sedgwick
Status: NEW → ASSIGNED

        Attachment #8349621 -
        Flags: review?(cade)

        Attachment #8349621 -
        Flags: feedback?(jon)

    
    

    
  


  

        Attachment #8349639 -
        Flags: review?(cade)

        Attachment #8349639 -
        Flags: feedback?(jon)

    
    

    
  
Comment on attachment 8349621 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/225

r+ with some fixes noted in the Pull Request

        Attachment #8349621 -
        Flags: review?(cade) → review+

    
    

    
  
Comment on attachment 8349639 [details] [review]
https://github.com/mozilla/webmaker.org/pull/567

r+ with the fixes noted in the pull request

        Attachment #8349639 -
        Flags: review?(cade) → review+

    
    

    
  


  

        Attachment #8349653 -
        Flags: review?(pomax)
Flags: needinfo?(jon)

    
  

        Attachment #8349621 -
        Flags: feedback?(jon)

    
  

        Attachment #8349639 -
        Flags: feedback?(jon)

    
    

    
  
Comment on attachment 8349653 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/333

r+ with a nit (no need for rereview if you fix it)

        Attachment #8349653 -
        Flags: review?(pomax) → review+

    
    

    
  
This needinfo was premature. I'll flag you again when this stuff has to go on staging.
Flags: needinfo?(jon)

    
    

    
  
:sedge - ping

    
    

    
  
Is this still relevant?
Flags: needinfo?(kieran.sedgwick)

    
    

    
  
Yes, I believe so. It's a simple patch, but I don't have a working dev environment to test it in. Would you be willing to pick it up?

Since it's changes an ENV variable, the config of the staging/production servers would need to be updated when it gets merged in.
Flags: needinfo?(kieran.sedgwick)

    
    

    
  
I can pick put this bug, if nobody against.

    
    

    
  
:adminx I assigned this to you. So basically most of Kieran's pull request are done, but you will need to update them and making sure they are tested locally and then coordinate with jbuck to have them merged in on staging/production.
Assignee: kieran.sedgwick → admix.snurnikov

    
    

    
  
Thanks Ali, got it. Basically, they were not merged and I need to rebase them, test and push carefully (after review of course) to master. And add more PRs to other components.

    
    

    
  


  
Changed vars in goggles. I will follow up on already approved PRs, they have to be rebased and updated.
PR -> https://github.com/mozilla/goggles.webmaker.org/pull/120

Q1: Possibly, maybe I could start with my new PR for those already r+, it will be much more easier?
Q2: Other components are:
      - login.webmaker.org
      - webmaker.org
      - thimble.webmaker.org
      - popcorn.webmaker.org
      - webmaker-events-2
 Did I miss something?

        Attachment #8395162 -
        Flags: review?(cade)

    
    

    
  
(In reply to Alexander [:admix] from comment #16)
> Created attachment 8395162 [details] [review]
> https://github.com/mozilla/goggles.webmaker.org/pull/120
> 
> Changed vars in goggles. I will follow up on already approved PRs, they have
> to be rebased and updated.
> PR -> https://github.com/mozilla/goggles.webmaker.org/pull/120
> 
> Q1: Possibly, maybe I could start with my new PR for those already r+, it
> will be much more easier?

You can close Kieran's pull request and resubmit yours if that will speed up your work :)


> Q2: Other components are:
>       - login.webmaker.org
>       - webmaker.org
>       - thimble.webmaker.org
>       - popcorn.webmaker.org
>       - webmaker-events-2
>  Did I miss something?

If the are using the same ENV name then you can also send a pull request for them as well I would say.

    
    

    
  


  
Changes in Popcorn.webmaker.org
PR -> https://github.com/mozilla/popcorn.webmaker.org/pull/522

        Attachment #8349621 -
        Attachment is obsolete: true

        Attachment #8349639 -
        Attachment is obsolete: true

        Attachment #8349653 -
        Attachment is obsolete: true

        Attachment #8395243 -
        Flags: review?(cade)

    
    

    
  


  
Changes in login.webmaker.org
PR -> https://github.com/mozilla/login.webmaker.org/pull/251

PS: Need to close the preious PR in login. (I don't have permissions)

        Attachment #8395246 -
        Flags: review?(cade)

    
    

    
  
Comment on attachment 8395162 [details] [review]
https://github.com/mozilla/goggles.webmaker.org/pull/120

There's one more line to fix up here - see the pull request.

        Attachment #8395162 -
        Flags: review?(cade) → review-

    
    

    
  
Comment on attachment 8395162 [details] [review]
https://github.com/mozilla/goggles.webmaker.org/pull/120

Fixed that line, and I found 1 more in the same file.

        Attachment #8395162 -
        Flags: review- → review?(cade)

    
    

    
  
Comment on attachment 8395245 [details] [review]
https://github.com/mozilla/webmaker.org/pull/643

It looks like we don't even use the HOSTNAME/APP_HOSTNAME variable in this project.

:jbuck - what do you think? remove it completely?

        Attachment #8395245 -
        Flags: review?(cade)

        Attachment #8395245 -
        Flags: review+

        Attachment #8395245 -
        Flags: feedback?(jon)

    
    

    
  
Comment on attachment 8395245 [details] [review]
https://github.com/mozilla/webmaker.org/pull/643

Yeah, just remove it entirely

        Attachment #8395245 -
        Flags: feedback?(jon) → feedback+

    
    

    
  
Comment on attachment 8395245 [details] [review]
https://github.com/mozilla/webmaker.org/pull/643

Removed HOSTNAME from README and env.dist.

        Attachment #8395245 -
        Flags: review+ → review?(cade)

    
    

    
  
These all look good to go - according to comment #2 jon wants to update all the configs :)
Flags: needinfo?(jon)

    
    

    
  
Done for:

* butter-staging
* goggles-staging
* thimble-staging
* webmakerorg-staging
* wmlogin-staging
* butter-production
* goggles-production
* thimble-production
* webmakerorg-production
* wmlogin-production

All done, land at will :admix!
Flags: needinfo?(jon)

    
    

    
  
Commit pushed to master at https://github.com/mozilla/webmaker.org

https://github.com/mozilla/webmaker.org/commit/c694af83b3df044d2bb552d6fad4c5b9daf27059
Fix [bug951709] - Removed 'HOSTNAME/APP_HOSTNAME' entirely from webmaker.org

    
    

    
  
We should let the webmaker Dev list know about this change

    
  
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED

    
  
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

    
    

    
  


  
Found a couple of lines in popcorn.webmaker.org with 'hostname' in config.js, changed to 'APP_HOSTNAME'
PR -> https://github.com/mozilla/popcorn.webmaker.org/pull/526

        Attachment #8398238 -
        Flags: review?(cade)

        Attachment #8398238 -
        Flags: review?(cade) → review+

    
  
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: