nativelink
nativelink copied to clipboard
NativeLink Should Not Start if CAS and AC are the Same
If users try to start NativeLink configuring CAS and AC to use the same store, it should fail and provide a helpful error.
It is related to: https://github.com/TraceMachina/nativelink/issues/744
I'd like to work on this issue. cc: @allada , @MarcusSorealheis
I resolved the issue , but as @aleksdmladenovic raised a pr already , I am not raising my pr now without your permission @MarcusSorealheis . But while reviewing his PR , I just want to suggest the modifications needed:
-
to reduce time complexity I created a HashMap called cas_store_map from the CAS configuration, where the key is the instance_name and the value is a reference to the cas_store.
-
then iterate over the AC configuration and check if the instance_name exists in the cas_store_map.
-
we compare the store names, and if they match, we return an error.
I have added a integration test file also in my solution simple_store_conflict_test.sh in the integration_tests directory
Want to know what I have written in my script?
1)A function check_store_conflict to simulate checking for conflicts between CAS and AC configurations.
2)Two test cases:
test_conflict_when_cas_and_ac_use_same_store to verify that a conflict is detected when CAS and AC use the same store.
test_no_conflict_when_cas_and_ac_use_different_stores to verify that no conflict is detected when CAS and AC use different stores.
The script runs both test cases and prints the result of each test.
@MarcusSorealheis need your permission to raise my PR , otherwise I can assist @aleksdmladenovic in his PR as being an active contributor , I always respect opensource rules
Raise a PR
Marcus Eagan
On Fri, Jul 19, 2024 at 11:33 DEBANJAN MUKHERJEE @.***> wrote:
I resolved the issue , but as @aleksdmladenovic https://github.com/aleksdmladenovic raised a pr already , I am not raising my pr now without your permission @MarcusSorealheis https://github.com/MarcusSorealheis . But while reviewing his PR , I just want to suggest the modifications needed:
to reduce time complexity I created a HashMap called cas_store_map from the CAS configuration, where the key is the instance_name and the value is a reference to the cas_store. 2.
then iterate over the AC configuration and check if the instance_name exists in the cas_store_map. 3.
we compare the store names, and if they match, we return an error.
I have added a integration test file also in my solution simple_store_conflict_test.sh in the integration_tests directory
Want to know what I have written in my script?
1)A function check_store_conflict to simulate checking for conflicts between CAS and AC configurations.
2)Two test cases: test_conflict_when_cas_and_ac_use_same_store to verify that a conflict is detected when CAS and AC use the same store. test_no_conflict_when_cas_and_ac_use_different_stores to verify that no conflict is detected when CAS and AC use different stores. The script runs both test cases and prints the result of each test.
@MarcusSorealheis https://github.com/MarcusSorealheis need your permission to raise my PR , otherwise I can assist @aleksdmladenovic https://github.com/aleksdmladenovic in his PR as being an active contributor , I always respect opensource rules
— Reply to this email directly, view it on GitHub https://github.com/TraceMachina/nativelink/issues/768#issuecomment-2239882290, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR6TSHAYJYK2IRY7BS4N5TZNFLXVAVCNFSM6AAAAABEZQZK7OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZZHA4DEMRZGA . You are receiving this because you were mentioned.Message ID: @.***>