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.

Sign up for GitHub

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

Jump to bottom

DM-47269: Change fail behavior of getSite() and fix build breakage #126

Merged
merged 2 commits into from
Oct 29, 2024
Merged

DM-47269: Change fail behavior of getSite() and fix build breakage #126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04f6f94
Change behaviour of getSite to return UNKNOWN rather than raise
mfisherlevine Oct 29, 2024
d05a360
Handle new unknown site behavior in efdUtils
mfisherlevine Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions python/lsst/summit/utils/butlerUtils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ def makeDefaultButler(
Raises
------
FileNotFoundError
Raised if the butler cannot be created.
Raised if the butler cannot be created, because this is the error
when the DAF_BUTLER_REPOSITORY_INDEX is not set correctly.
"""
SUPPORTED_SITES = [
"summit",
Expand All @@ -186,7 +187,12 @@ def makeDefaultButler(

site = getSite()
if site not in SUPPORTED_SITES:
raise ValueError(f"Default butler creation only supported at sites: {SUPPORTED_SITES}, got {site=}")
# This might look like a slightly weird error to raise, but this is
# the same error that's raised when the DAF_BUTLER_REPOSITORY_INDEX
# isn't set, so it's the most appropriate error to raise here, i.e.
# this is what would be raised if this function was allowed to continue
# only this is a less confusing version of it.
raise FileNotFoundError(f"Default butler creation only available at: {SUPPORTED_SITES}, got {site=}")

summitLike = site in ["summit", "tucson", "base"]
if summitLike:
Expand Down
7 changes: 3 additions & 4 deletions python/lsst/summit/utils/efdUtils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,10 +453,9 @@ def makeEfdClient(testing: bool | None = False) -> EfdClient:
if testing:
return EfdClient("usdf_efd")

try:
site = getSite()
except ValueError as e:
raise RuntimeError("Could not create EFD client as the site could not be determined") from e
site = getSite()
if site == "UNKNOWN":
raise RuntimeError("Could not create EFD client as the site could not be determined")

if site == "summit":
return EfdClient("summit_efd")
Expand Down
7 changes: 2 additions & 5 deletions python/lsst/summit/utils/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,11 +600,8 @@ def getSite() -> str:
One of:
['tucson', 'summit', 'base', 'staff-rsp', 'rubin-devl', 'jenkins',
'usdf-k8s']
If the location cannot be determined "UNKOWN" is returned.

Raises
------
ValueError
Raised if location cannot be determined.
"""
# All nublado instances guarantee that EXTERNAL_URL is set and uniquely
# identifies it.
Expand Down Expand Up @@ -647,7 +644,7 @@ def getSite() -> str:
return "usdf-k8s"

# we have failed
raise ValueError("Location could not be determined")
return "UNKOWN"
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that butlerUtils and efdUtils each contain a try..except block that checks for ValueError. Suggest modifying those to remove the try..except or just ticketing the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Changed one, and ticketed the one that appears to be fine and I'm too scared to touch right now!



def getAltAzFromSkyPosition(
Expand Down
Loading