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.

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

Issue 141 -- fix 'no agent available' user view #166

Merged
merged 8 commits into from
May 31, 2024

Conversation

tskluzac
Copy link
Collaborator

When a local agent is unavailable to relay messages from the client to Zambeze, the user experience should be 'informative and helpful'. This branch is a first attempt at improving that user workflow.

Closes #141

@wigging
Copy link
Collaborator

wigging commented May 24, 2024

The dispatch, status, and result methods on the Campaign class need docstrings. Other than that this looks fine.

@tskluzac
Copy link
Collaborator Author

Added docstring to dispatch. Removed status and result as those are improperly implemented and partially moved to other classes. Cleaning up as we go :)

@tskluzac
Copy link
Collaborator Author

@wigging please take a look when you get the chance (I forgot to tag you on Friday)

@wigging
Copy link
Collaborator

wigging commented May 29, 2024

It looks like you used the Google style for the docstrings. I thought we decided to use the NumPy style? I also suggested the NumPy style in the CONTRIBUTING.md document. So which style should we use?

@wigging
Copy link
Collaborator

wigging commented May 30, 2024

You also need to rebase this branch so it's up-to-date with the main branch.

@tskluzac
Copy link
Collaborator Author

@wigging I believe this is proper numpy style now (per their docs that you linked in the contributing doc). That's what I get for asking AI for helping in the docstring conversions...

@wigging
Copy link
Collaborator

wigging commented May 31, 2024

Indentation is screwed up.

"""Initializes a Campaign instance.

Parameters
----------
    name : str
        The name of the campaign.
    activities : str, optional
        List of science activities for processing by Zambeze.
    logger : logging.Logger, optional
        Logger object to flush stderr and stdout
    force_login : bool
        Boolean whether to force a Globus Auth flow.
"""

Should be like this:

"""Initializes a Campaign instance.

Parameters
----------
name : str
    The name of the campaign.
activities : str, optional
    List of science activities for processing by Zambeze.
logger : logging.Logger, optional
    Logger object to flush stderr and stdout
force_login : bool
    Boolean whether to force a Globus Auth flow.
"""

Same for the other docstrings. Also, Notes: should be underlined and remove the : at the end.

All of this can be checked by the linter. But if we enable it in GitHub Actions then most of the Python files would fail because the docstrings are a mess in this project.

@wigging wigging merged commit 9008483 into main May 31, 2024
1 check passed
@wigging wigging deleted the skluzacek_141_unavailable_fix branch May 31, 2024 16:42
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.

Should gracefully notify users that no agent is available...
2 participants