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

Add status parameter to filter jobs #30

Merged
merged 26 commits into from
Jun 3, 2022

Conversation

HudZah
Copy link
Contributor

@HudZah HudZah commented May 27, 2022

Context:
With this change users can filter the jobs by the status parameter that is passed to the / jobs endpoint where the jobs are filtered and returned.

Description of the Change:

  • Added status parameter to the xcc.Job.list()
  • Accepts the status parameter in the CLI as well by using the --status argument.
  • Added tests to ensure that it works as intended
response = connection.request(
            "GET", "/jobs", params={"size": size, "id": ids, "status": status}
        )

Benefits:

  • Easier filteration of jobs to view them solely based on the input status.

Possible Drawbacks:
N/A

Related GitHub Issues:
N/A

@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name and GitHub username in the contributors section.

Copy link
Contributor

@doctorperceptron doctorperceptron left a comment

Choose a reason for hiding this comment

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

Looks good. We should probably include a list of available statuses so users don't have to guess at what they could be.

.github/CHANGELOG.md Outdated Show resolved Hide resolved
HudZah and others added 7 commits May 31, 2022 13:43
Co-authored-by: Paul Finlay <50180049+doctorperceptron@users.noreply.github.com>
…b.com:XanaduAI/xanadu-cloud-client into sc-19591-add-status-parameter-to-job-list-api
@doctorperceptron doctorperceptron self-requested a review May 31, 2022 20:56
Copy link
Collaborator

@Mandrenkov Mandrenkov 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 contribution, @HudZah! 🎉

What happens if ids and status are both specified? 🤔

  • Does ids take precedence?
  • Or will status filter the jobs specified by ids?

.github/CHANGELOG.md Outdated Show resolved Hide resolved
.github/CHANGELOG.md Outdated Show resolved Hide resolved
.github/CHANGELOG.md Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
docs/requirements.txt Show resolved Hide resolved
xcc/job.py Outdated Show resolved Hide resolved
xcc/job.py Outdated Show resolved Hide resolved
xcc/job.py Outdated Show resolved Hide resolved
tests/test_job.py Outdated Show resolved Hide resolved
tests/test_job.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Mandrenkov Mandrenkov left a comment

Choose a reason for hiding this comment

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

The latter, I tested this previously, and it should filter the jobs specified by ids I believe.

Ah, okay! That seems pretty intuitive. 👍

I was going to suggest documenting this behaviour in the xcc job list help documentation and xcc.Job.list docstring but, upon further reflection, I don't think that is necessary.

.github/CHANGELOG.md Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
docs/requirements.txt Outdated Show resolved Hide resolved
tests/test_job.py Outdated Show resolved Hide resolved
tests/test_job.py Outdated Show resolved Hide resolved
xcc/job.py Outdated Show resolved Hide resolved
HudZah and others added 8 commits June 2, 2022 23:22
Co-authored-by: Mikhail Andrenkov <Mandrenkov@users.noreply.github.com>
Co-authored-by: Mikhail Andrenkov <Mandrenkov@users.noreply.github.com>
Co-authored-by: Mikhail Andrenkov <Mandrenkov@users.noreply.github.com>
Co-authored-by: Mikhail Andrenkov <Mandrenkov@users.noreply.github.com>
Co-authored-by: Mikhail Andrenkov <Mandrenkov@users.noreply.github.com>
Co-authored-by: Mikhail Andrenkov <Mandrenkov@users.noreply.github.com>
Co-authored-by: Mikhail Andrenkov <Mandrenkov@users.noreply.github.com>
@HudZah HudZah merged commit 2c0e6b7 into main Jun 3, 2022
@HudZah HudZah deleted the sc-19591-add-status-parameter-to-job-list-api branch June 3, 2022 15:16
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