Label postgresql full text search #272
Conversation
…-d-sharma/cmf into label_postgresql_full_text_search
|
|
||
| async getLabelsList() { | ||
| return this.apiClient | ||
| .get(`/api/labels/status`) |
There was a problem hiding this comment.
we can't add 'api' here. it breaks the pattern of rest api we are using.
|
|
||
| async searchLabelArtifacts(pipeline_name, content_filter, sort_order = "asc", active_page = 1, record_per_page = 5) { | ||
| return this.apiClient | ||
| .get(`/artifacts/${pipeline_name}/Label/search`, { |
There was a problem hiding this comment.
make 'L' in the 'Label' small like 'label'
| } else if (selectedPipeline === null && pipelines.length === 0) { | ||
| setLoading(false); | ||
| } | ||
| }, [selectedPipeline, pipelines.length]); |
There was a problem hiding this comment.
Explain why checking .length is necessary.
| }); | ||
| }; | ||
|
|
||
| const fetchArtifacts = async (pipelineName, artifactType, sortOrder, activePage, filter="", selectedCol) => { |
There was a problem hiding this comment.
Add appropriate comment for easy understanding. and colud you explain why async is used?
| @app.get("/artifacts/{pipeline_name}/Label/search") | ||
| async def search_label_artifacts( | ||
| pipeline_name: str, | ||
| content_filter: str = Query(..., description="Search term to find in label content"), |
There was a problem hiding this comment.
Here you can use Pydantic model.
|
|
||
| # Find all structured conditions | ||
| matches = cls.CONDITION_PATTERN.findall(query) | ||
| matched_positions = [] |
There was a problem hiding this comment.
Why matched_position used?
| raise HTTPException(status_code=500, detail=f"Error searching label artifacts: {str(e)}") | ||
|
|
||
|
|
||
| # api to display executions available in mlmd file[from postgres] |
There was a problem hiding this comment.
There are two execution function.
| raise HTTPException(status_code=500, detail=f"Reindexing failed: {str(e)}") | ||
|
|
||
| @app.get("/api/labels/status") | ||
| async def get_label_search_status(db: AsyncSession = Depends(get_db)): |
There was a problem hiding this comment.
We need to understand where to use this api.
| plain_terms=plain_terms if plain_terms else None, | ||
| conditions=conditions if conditions else None, | ||
| pipeline_name=pipeline_name, | ||
| limit=100 |
There was a problem hiding this comment.
What do we mean by limit 100 here?
| if plain_terms: | ||
| # Only search properties if there are plain text terms | ||
| property_search_results = await fetch_artifacts( | ||
| db, pipeline_name, "Label", " ".join(plain_terms), 1, 1000, "name", sort_order |
There was a problem hiding this comment.
As per my understanding 1 is active page and 1000 is record_per_page this is not correct as we want search to be performed on full data. not just on the current page. If plain _terms is true, it is essentially our existing functionality it should work same.
| label_file = result.get('label_file', '') | ||
| if label_file: | ||
| # Remove file extension if present | ||
| clean_label_file = label_file.replace('.csv', '') if label_file.endswith('.csv') else label_file |
There was a problem hiding this comment.
Could you explain why we are replacing file extension.
…-d-sharma/cmf into label_postgresql_full_text_search
3 participants
Related Issues / Pull Requests
List all related issues and/or pull requests if there are any.
Description
Include a brief summary of the proposed changes.
What changes are proposed in this pull request?
examples in this repository need to be updated too).
Checklist:
uses Google-style formatting and any other formatting that is supported by mkdocs and plugins this project
uses.