Skip to content

AgamaProposal: Honor DiskAnalyzer candidate devices when searching drives#1765

Merged
ancorgs merged 2 commits intoagama-project:masterfrom
ancorgs:only_candidates
Nov 14, 2024
Merged

AgamaProposal: Honor DiskAnalyzer candidate devices when searching drives#1765
ancorgs merged 2 commits intoagama-project:masterfrom
ancorgs:only_candidates

Conversation

@ancorgs
Copy link
Contributor

@ancorgs ancorgs commented Nov 14, 2024

Problem

When searching drives, the initial implementation of AgamaProposal uses all disk (or disk-like devices, to be precise) in the system.

That is ok as a first attempt, but we always knew we might need to refine that a bit.

Now that we are working on the new interface for storage configuration, fully based on AgamaProposal, the fact that AgamaProposal can choose a disk that is not considered by DiskAnalyzer to be an installation candidate feels a bit odd.

Solution

For the time being (and with minimal code changes), this adapts the AgamaProposal algorithm to filter out disks that are skipped by DiskAnalyzer. That typically means the disks from which the installer is running, disks that contain installation repositories, etc.

We may need to refine that in the future again. But for now that's the most consistent behavior we can adopt without taking big decisions.

Testing

Added a new unit test

Manual testing done by @joseivanlopez.

@coveralls
Copy link

coveralls commented Nov 14, 2024

Pull Request Test Coverage Report for Build 11838176757

Details

  • 12 of 12 (100.0%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.02%) to 71.35%

Files with Coverage Reduction New Missed Lines %
service/lib/agama/storage/manager.rb 1 97.1%
service/service/lib/agama/storage/manager.rb 1 97.1%
Totals Coverage Status
Change from base Build 11831842629: 0.02%
Covered Lines: 16915
Relevant Lines: 23707

💛 - Coveralls

Copy link
Contributor

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@ancorgs ancorgs merged commit bce4d54 into agama-project:master Nov 14, 2024
@imobachgs imobachgs mentioned this pull request Jan 10, 2025
imobachgs added a commit that referenced this pull request Jan 13, 2025
Update to release version 11.

* #1495
* #1564
* #1617
* #1618
* #1625
* #1626
* #1627
* #1628
* #1630
* #1631
* #1632
* #1633
* #1634
* #1635
* #1636
* #1639
* #1640
* #1641
* #1642
* #1643
* #1644
* #1645
* #1646
* #1647
* #1648
* #1649
* #1650
* #1651
* #1652
* #1654
* #1655
* #1656
* #1657
* #1660
* #1663
* #1666
* #1667
* #1668
* #1670
* #1671
* #1673
* #1674
* #1675
* #1676
* #1677
* #1681
* #1682
* #1683
* #1684
* #1687
* #1688
* #1689
* #1690
* #1691
* #1692
* #1693
* #1694
* #1695
* #1696
* #1698
* #1699
* #1702
* #1703
* #1704
* #1705
* #1707
* #1708
* #1709
* #1710
* #1711
* #1712
* #1713
* #1714
* #1715
* #1716
* #1717
* #1718
* #1720
* #1721
* #1722
* #1723
* #1727
* #1728
* #1729
* #1731
* #1732
* #1733
* #1734
* #1735
* #1736
* #1737
* #1740
* #1741
* #1743
* #1744
* #1745
* #1746
* #1751
* #1753
* #1754
* #1755
* #1757
* #1762
* #1763
* #1764
* #1765
* #1766
* #1767
* #1769
* #1771
* #1772
* #1773
* #1774
* #1777
* #1778
* #1785
* #1786
* #1787
* #1788
* #1789
* #1790
* #1791
* #1792
* #1793
* #1794
* #1795
* #1796
* #1797
* #1798
* #1799
* #1800
* #1802
* #1803
* #1804
* #1805
* #1807
* #1808
* #1809
* #1810
* #1811
* #1812
* #1814
* #1815
* #1821
* #1822
* #1823
* #1824
* #1825
* #1826
* #1827
* #1828
* #1830
* #1831
* #1832
* #1833
* #1834
* #1835
* #1836
* #1837
* #1838
* #1839
* #1840
* #1841
* #1842
* #1843
* #1844
* #1845
* #1847
* #1848
* #1849
* #1850
* #1851
* #1854
* #1855
* #1856
* #1857
* #1860
* #1861
* #1863
* #1864
* #1865
* #1866
* #1867
* #1871
* #1872
* #1873
* #1875
* #1876
* #1877
* #1878
* #1880
* #1881
* #1882
* #1883
* #1884
* #1885
* #1886
* #1888
* #1889
* #1890
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