-
Notifications
You must be signed in to change notification settings - Fork 4
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
Nersc deployment adaptations (Auth, Environment variables) #124
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
69be61d
Ignore differently named Python envs
Wiebke 5edaa6e
Rename env-key for Tiled access
Wiebke 64156fc
Make `EXPORT_FILE_PATH` an environment variable
Wiebke 37cd758
Add basic dash auth
Wiebke 8be41bc
Add constants file to docker-compose
Wiebke 31cfb26
Update Readme with new Tiled URI and additional variables
Wiebke fdff0d5
:passport_control: Disable authentication for local testing
Wiebke 3b300ba
Use a script to startup local Tiled server with an api key
Wiebke File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,12 +15,12 @@ pri-venv | |
data/ | ||
|
||
env | ||
*env/ | ||
*.hdf5 | ||
*.db | ||
|
||
.theia/ | ||
*.tiff | ||
*.tif | ||
|
||
.env | ||
config.py | ||
.env |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
#!/bin/bash | ||
source .env | ||
export TILED_SINGLE_USER_API_KEY=$TILED_API_KEY | ||
tiled serve directory data |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm testing this against our local Tiled server as well and get surprising behaviour with this change. My Python code throws an authentication error:
And the local Tiled server blocks my requests:
This seems to be only the case if this variable is called
TILED_API_KEY
. Not a problem, for example, if I change this toTLD_API_KEY
. Wondering if this is because Tiled is trying to be too smart under the hood and something in the local client detects any env var with this name and tries to apply it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, you are right. My current understanding of what causes this error is as follows:
The client picks up on
TILED_API_KEY
being set as an environment variables and then uses that as an API key to authenticate, see [tiled/client/context.py#L79].(https://github.com/bluesky/tiled/blob/7566d0f2d61cb0477bf187b33e94d7449358c4e5/tiled/client/context.py#L79)Our local server started with
tiled serve directory --public data
is in principle accessible without a key (we are allowed to read data without having an api_key in the request), but it does generate a key on startup (which would be needed for writing data). Our requests having a different key then causes the error.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see 2 main ways to address this:
TILED_API_KEY
environment variable to something else, orKeeping in mind that we may have this application run in different deployment environments and hooked up to different Tiled servers, I think I would like to stay away from baking in more semantics into the variable. In the latest update, I thus added a script that starts the local server with the
TILED_API_KEY
from the environment file by setting theTILED_SINGLE_API_KEY
variable that the server makes use of (if set).Let me know if that works for your test workflow @hannahker.