Skip to content
This repository has been archived by the owner on May 22, 2023. It is now read-only.

Add GIRDER_LOCAL_URL environment variable #25

Merged
merged 1 commit into from
Aug 25, 2020
Merged

Add GIRDER_LOCAL_URL environment variable #25

merged 1 commit into from
Aug 25, 2020

Conversation

jwodder
Copy link
Member

@jwodder jwodder commented Aug 11, 2020

Needed for dandi/dandi-cli#186

@satra
Copy link
Member

satra commented Aug 11, 2020

@jwodder - is this a make my life easier PR for dandi-cli or do you actually need access to both instances simultaneously.

i can easily imagine:

SET GIRDER URL 1: run tests
SET GIRDER URL 2: run tests
...
unless there is a test that needs both URLs simultaneously.

@jwodder
Copy link
Member Author

jwodder commented Aug 11, 2020

It's for accessing the same instance of Girder through different URLs. dandi-cli's tests involve a Docker Compose setup with a Girder container and a redirector container. The redirector container has to access Girder via the url http://girder:8080, yet responses from /server-info have to use the URL http://localhost:8081. Neither URL is valid in the other one's context, so we need a way to tell serve.py to use one URL when accessing Girder and another when reporting Girder's URL via /server-info.

@yarikoptic
Copy link
Member

yarikoptic commented Aug 11, 2020

a possible "freshly discovered" alternative which might work is a host mode for network (yet to be tested to work elsewhere but my laptop) but then ideally we would need to be able to tune up the port of redirector instead of hardcoded popular 8080... might be worth doing that (adding e.g. DANDI_REDIRECTOR_PORT) anyways I guess... then we hopefully would stop pestering redirector for a longer period of time ;-)

edit: may be could be REDIRECTOR_URL and then code would parse out possible port... redirector ATM does not reveal its own URL, and I hope it would never need to, but might be worth getting ready for that? might want to handle possible https:// protocol spec? sorry for piling up.... I am happy with any solution which would get us forward

@yarikoptic
Copy link
Member

@satra ping. this PR is waiting on your blessing or cursing... AFAIK we found no alternative insofar and would need it to establish more realistic testing within dandi-cli

@satra
Copy link
Member

satra commented Aug 25, 2020

@yarikoptic - i'll merge this for now, but let's see if we can figure out a better solution.

@satra satra merged commit 4ebf8db into dandi:master Aug 25, 2020
@jwodder jwodder deleted the girder-local-url branch November 21, 2020 03:21
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