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

Feature: support search_paths option in QGIS finder job #535

Merged
merged 14 commits into from
Sep 6, 2024

Conversation

Guts
Copy link
Collaborator

@Guts Guts commented Aug 30, 2024

Close #523

Search of QGIS installed binary is now done recursively from a list of search paths.

@github-actions github-actions bot added enhancement New feature or request jobs Scenarios and jobs labels Aug 30, 2024
@codecov-commenter
Copy link

codecov-commenter commented Aug 30, 2024

Codecov Report

Attention: Patch coverage is 24.56140% with 43 lines in your changes missing coverage. Please review.

Project coverage is 71.10%. Comparing base (5c8372c) to head (b2477a5).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...ment_toolbelt/jobs/job_qgis_installation_finder.py 20.37% 43 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #535      +/-   ##
==========================================
+ Coverage   71.07%   71.10%   +0.02%     
==========================================
  Files          47       47              
  Lines        3125     3135      +10     
  Branches      669      671       +2     
==========================================
+ Hits         2221     2229       +8     
- Misses        714      716       +2     
  Partials      190      190              
Flag Coverage Δ
unittests 70.52% <24.56%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
qgis_deployment_toolbelt/constants.py 86.17% <100.00%> (+0.45%) ⬆️
...ment_toolbelt/jobs/job_qgis_installation_finder.py 34.81% <20.37%> (+2.00%) ⬆️

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Aug 30, 2024
@Guts Guts force-pushed the feature/job-qgis-finder-search-paths branch from de09890 to c1f83ef Compare August 30, 2024 15:21
@jmkerloch
Copy link
Collaborator

jmkerloch commented Aug 30, 2024

It will be better to support searching in subdirectory for search_paths option. Check with Path().rglob()

@Guts Guts force-pushed the feature/job-qgis-finder-search-paths branch 2 times, most recently from 0997bfa to 15f982a Compare September 5, 2024 06:53
@github-actions github-actions bot added the quality Tests, project resiliency, etc. label Sep 5, 2024
@jmkerloch jmkerloch marked this pull request as ready for review September 5, 2024 08:28
@Guts
Copy link
Collaborator Author

Guts commented Sep 5, 2024

@jmkerloch Does setting search_path overwrite the default paths (program files/qgis X.y.z and OGESO4W) or not?

If not, shouldn't we consider that these are the default values for 'search_paths'? In this way, a user may want QDT to search in a particular location (D://...) without looking in C:/program files/qgis X.y.z and OGESO4W.

@Guts
Copy link
Collaborator Author

Guts commented Sep 5, 2024

I think there is something to improve regarding version comparison.

Test case:

scenario:.qdt.yml

  - name: Find installed QGIS
    uses: qgis-installation-finder
    with:
      version_priority:
        - "3.40"
        - "3.34"
        - "3.28"
        - "3.38"
        - "3.36"
        - "3.32"
      search_paths:
        - "%PROGRAMFILES%/QGIS"
      if_not_found: warn

I've installed a QGIS 3.34.9 and the job found it as expected:

2024-09-05 14:51:04||DEBUG||job_qgis_installation_finder||run_needed||135||'QDT_QGIS_EXE_PATH' is not defined. Searching for QGIS executable is necessary.
2024-09-05 14:51:04||DEBUG||job_qgis_installation_finder||_get_qgis_versions_in_dir||237||Searching for QGIS binary in C:\OSGeo4W with pattern ['qgis-bin.exe', 'qgis-ltr-bin.exe']
2024-09-05 14:51:04||DEBUG||job_qgis_installation_finder||_get_qgis_versions_in_dir||237||Searching for QGIS binary in C:\Program Files\QGIS 3.34.4 with pattern ['qgis-bin.exe', 'qgis-ltr-bin.exe']     
2024-09-05 14:51:04||DEBUG||job_qgis_installation_finder||_get_qgis_versions_in_dir||237||Searching for QGIS binary in C:\Program Files/QGIS with pattern ['qgis-bin.exe', 'qgis-ltr-bin.exe']
2024-09-05 14:51:06||DEBUG||job_qgis_installation_finder||_search_qgis_version_and_add_to_dict||330||QGIS version 3.34.9 found : C:\Program Files\QGIS\3_34\bin\qgis-ltr-bin.exe
2024-09-05 14:51:06||DEBUG||job_qgis_installation_finder||get_installed_qgis_path||156||Found installed QGIS: {'3.34.9': 'C:\\Program Files\\QGIS\\3_34\\bin\\qgis-ltr-bin.exe'}

But then, it complains about a non matching version:

2024-09-05 14:51:06||INFO||job_qgis_installation_finder||get_installed_qgis_path||177||QGIS version(s) [3.40,3.34,3.28,3.38,3.36,3.32] not found. Using most recent found version 3.34.9 : C:\Program Files\QGIS\3_34\bin\qgis-ltr-bin.exe

Even if I use a more accurate version number in scenario, it says the same thing:

  - name: Find installed QGIS
    uses: qgis-installation-finder
    with:
      version_priority:
        - "3.40"
        - "3.34.9"
        - "3.28"
        - "3.38"
        - "3.36"
        - "3.32"
      search_paths:
        - "%PROGRAMFILES%/QGIS"
      if_not_found: warning
2024-09-05 15:02:51||DEBUG||job_qgis_installation_finder||run_needed||135||'QDT_QGIS_EXE_PATH' is not defined. Searching for QGIS executable is necessary.
2024-09-05 15:02:51||DEBUG||job_qgis_installation_finder||_get_qgis_versions_in_dir||237||Searching for QGIS binary in C:\OSGeo4W with pattern ['qgis-bin.exe', 'qgis-ltr-bin.exe']
2024-09-05 15:02:51||DEBUG||job_qgis_installation_finder||_get_qgis_versions_in_dir||237||Searching for QGIS binary in C:\Program Files\QGIS 3.34.4 with pattern ['qgis-bin.exe', 'qgis-ltr-bin.exe']
2024-09-05 15:02:51||DEBUG||job_qgis_installation_finder||_get_qgis_versions_in_dir||237||Searching for QGIS binary in C:\Program Files/QGIS with pattern ['qgis-bin.exe', 'qgis-ltr-bin.exe']
2024-09-05 15:02:52||DEBUG||job_qgis_installation_finder||_search_qgis_version_and_add_to_dict||330||QGIS version 3.34.9 found : C:\Program Files\QGIS\3_34\bin\qgis-ltr-bin.exe
2024-09-05 15:02:52||DEBUG||job_qgis_installation_finder||get_installed_qgis_path||156||Found installed QGIS: {'3.34.9': 'C:\\Program Files\\QGIS\\3_34\\bin\\qgis-ltr-bin.exe'}
2024-09-05 15:02:52||INFO||job_qgis_installation_finder||get_installed_qgis_path||177||QGIS version(s) [3.40,3.34.9,3.28,3.38,3.36,3.32] not found. Using most recent found version 3.34.9 : C:\Program Files\QGIS\3_34\bin\qgis-ltr-bin.exe
2024-09-05 15:02:52||DEBUG||job_qgis_installation_finder||run||103||qgis-installation-finder : QDT_QGIS_EXE_PATH is now C:\Program Files\QGIS\3_34\bin\qgis-ltr-bin.exe
2024-09-05 15:02:52||DEBUG||job_qgis_installation_finder||run||112||Job qgis-installation-finder ran successfully.

@jmkerloch
Copy link
Collaborator

@Guts for the error with version comparison, it comes from an invalid log. This commit fix this : 3bfa533

@jmkerloch
Copy link
Collaborator

@jmkerloch Does setting search_path overwrite the default paths (program files/qgis X.y.z and OGESO4W) or not?

If not, shouldn't we consider that these are the default values for 'search_paths'? In this way, a user may want QDT to search in a particular location (D://...) without looking in C:/program files/qgis X.y.z and OGESO4W.

For the specific search in C:/program files/QGIS x.y.z this can't be done with the search path option because we are using a regexp to get the QGIS x.y.z folders.

@Guts
Copy link
Collaborator Author

Guts commented Sep 5, 2024

@jmkerloch Does setting search_path overwrite the default paths (program files/qgis X.y.z and OGESO4W) or not?
If not, shouldn't we consider that these are the default values for 'search_paths'? In this way, a user may want QDT to search in a particular location (D://...) without looking in C:/program files/qgis X.y.z and OGESO4W.

For the specific search in C:/program files/QGIS x.y.z this can't be done with the search path option because we are using a regexp to get the QGIS x.y.z folders.

Ok. If so, we should disable default locations if search_paths is not None.

@Guts
Copy link
Collaborator Author

Guts commented Sep 5, 2024

Once it's done, LGTM.

@jmkerloch jmkerloch self-assigned this Sep 5, 2024
@jmkerloch jmkerloch added this to the 0.35.3 milestone Sep 6, 2024
@jmkerloch jmkerloch force-pushed the feature/job-qgis-finder-search-paths branch from 5d3ece1 to b2477a5 Compare September 6, 2024 09:02
Copy link

sonarcloud bot commented Sep 6, 2024

@jmkerloch jmkerloch merged commit 0e4e6d1 into main Sep 6, 2024
26 checks passed
@jmkerloch jmkerloch deleted the feature/job-qgis-finder-search-paths branch September 6, 2024 09:16
@Guts Guts linked an issue Sep 6, 2024 that may be closed by this pull request
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request jobs Scenarios and jobs quality Tests, project resiliency, etc.
Projects
None yet
3 participants