Skip to content

[batch] Improve inactive client UX#15298

Open
cjllanwarne wants to merge 7 commits intohail-is:mainfrom
cjllanwarne:cjl_inactive_client_ux
Open

[batch] Improve inactive client UX#15298
cjllanwarne wants to merge 7 commits intohail-is:mainfrom
cjllanwarne:cjl_inactive_client_ux

Conversation

@cjllanwarne
Copy link
Collaborator

Change Description

Wires through the not authenticated message for hailtop.batch users who are unauthenticated or inactive.

Before: See #Hail Batch support > Hail Batch issue with submission @ 💬

After:

Traceback (most recent call last):
  File ".../hello.py", line 3, in <module>
    b = hb.Batch('hello test')
        ^^^^^^^^^^^^^^^^^^^^^^
  File ".../hail/python/hailtop/batch/batch.py", line 197, in __init__
    self._backend = ServiceBackend()
                    ^^^^^^^^^^^^^^^^
  File ".../hail/python/hailtop/batch/backend.py", line 645, in __init__
    regions = [ServiceBackend.default_region()]
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../hail/python/hailtop/batch/backend.py", line 569, in default_region
    raise e.with_traceback(None) from None
hailtop.batch_client.aioclient.BatchNotAuthenticatedError: Not authenticated with Hail Batch.

Please run:

    hailctl auth login

to obtain credentials. If problems persist, try logging in to the web UI to check account status.

Note: It would probably be even better to return the inactive state directly via the APIs, so we don't have to say "or try logging in to the web UI", but that's probably a much bigger change

Security Assessment

Delete all except the correct answer:

  • This change potentially impacts the Hail Batch instance as deployed by Broad Institute in GCP

Impact Rating

  • This change has a low security impact

Impact Description

Better catching of inactive errors in the client

Appsec Review

  • Required: The impact has been assessed and approved by appsec

@cjllanwarne cjllanwarne requested a review from grohli February 23, 2026 19:20
@cjllanwarne cjllanwarne requested a review from a team as a code owner February 23, 2026 19:20
)

self.__fs = self._requester_pays_fses[None]
self.__batch_client: Optional[AioBatchClient] = None
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moving this earlier means if the default region check fails, we don't also get a whole bunch of junk in the log because resource cleanup failed too because the batch client wasn't available

Copy link
Contributor

@grohli grohli left a comment

Choose a reason for hiding this comment

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

Thanks for the improvement, LGTM!

@cjllanwarne cjllanwarne self-assigned this Mar 3, 2026
return dummy_client.default_region()
except (BatchNotAuthenticatedError, PermissionError) as e:
# Reduce stack trace here. Users won't care where this came from. They care they're not authenticated.
raise e.with_traceback(None) from None
Copy link

@sarahgibs sarahgibs Mar 3, 2026

Choose a reason for hiding this comment

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

What ends up being returned to the client? What does the error message look like?

I'm guessing it's this error text a couple files down in this PR?
"Not authenticated with Hail Batch.\n\nPlease run:\n\n hailctl auth login\n\nto obtain credentials. If problems persist, try logging in to the web UI to check account status."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, exactly. You can see an example in the description

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants