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

Update prefect query functions #177

Merged
merged 6 commits into from
Mar 8, 2024
Merged

Update prefect query functions #177

merged 6 commits into from
Mar 8, 2024

Conversation

taxe10
Copy link
Member

@taxe10 taxe10 commented Mar 8, 2024 •
edited
Loading

This PR updated the query-related functions according to mlexchange/mlex_prefect_worker#16 In particular, it adds:

  • Retrieving children flow run ids according to a given parent flow run id. These are sorted according to START_TIME_ASC by default, with the child flow that started first being at the beginning of the list
  • Minor change to query function, such that it can be used for both (1) retrieving flows by names, and (2) querying children flows

@taxe10 taxe10 requested a review from Wiebke March 8, 2024 08:41
Wiebke
Wiebke approved these changes Mar 8, 2024
View reviewed changes
Copy link
Member

@Wiebke Wiebke left a comment

Choose a reason for hiding this comment

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

This is great!

I had two small questions, but are they are not an obstacle to merging this.

utils/prefect.py Show resolved Hide resolved
utils/prefect.py
async def _flow_run_query(tags, flow_run_name=None):
flow_runs_by_name = []
async def _flow_run_query(
tags=None, flow_run_name=None, parent_flow_run_id=None, sort="START_TIME_DESC"
Copy link
Member

Choose a reason for hiding this comment

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

I do not think we will ever use the function in this way, but what happens if all parameters are None, do we filter nothing and retrieve all flows?

Copy link
Member Author

Choose a reason for hiding this comment

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

It retrieves all flows, I just did a quick test at my end.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, good to know!

Wiebke
Wiebke approved these changes Mar 8, 2024
View reviewed changes
Copy link
Member

@Wiebke Wiebke left a comment

Choose a reason for hiding this comment

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

Nice, being able to trust the project_name in the tag makes logic around result retrieval simpler too.

@Wiebke Wiebke merged commit fbab404 into mlexchange:main Mar 8, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wiebke Wiebke Wiebke approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@taxe10 @Wiebke