feat: Add working-directory to the target, group and global scope #65

abhijeetSaroha · 2023-11-29T05:36:54Z

Changes Made:

Modified the _call_shell_app and _load_working_directory methods to address the working directory issue.

Details:

Refactored the _call_shell_app method to dynamically set the working directory based on the target, group, or global scope.
Updated the _load_working_directory method to handle both absolute and relative paths appropriately.
The _call_shell_app method now utilizes the correct working directory based on the priority of target, group, and global settings.
The _load_working_directory method has been improved to handle various scenarios for setting the working directory.

Solves #48

abhijeetSaroha · 2023-11-29T05:37:27Z

Heyy @xmnlab , can you please review this?

xmnlab

in _call_shell_app function: https://github.com/osl-incubator/makim/pull/65/files#diff-c70963049bfd82892ce83a4cc6a34fa99a9975e7cbbe93418a2d2fcfa2ed405aR94-R104

you shouldn't be doing this there. _load_working_directory should be handling all the step for setting a variable in the class with the value of the working_directory to be used.

it is kind of the idea of single responsible one function should be responsible for just one thing.

for env we have the following class attributes:

    env: dict = {}  # initial env
    env_scoped: dict = {}  # current env

so I think you can have something similar:

    working_directory: dict = {}  # initial working_directory
    working_directory_scoped: dict = {}  # current working_directory

additionally, pay attention in the order of the resolution for the working directory. in a makim file you can have the working directory in the global scope, group scope and target scope .. you should resolve that in that order ... so instead of if/elif ... you should use just if for each one ... you can use _load_scoped_data as a reference about how to resolve that in the correct order

I know that it is too much information, but let me know if you have any questions

abhijeetSaroha · 2023-11-30T13:38:55Z

Improved the logic for determining the working directory, making the code more readable and maintainable.
Set the scoped working directory appropriately based on the resolved path.

xmnlab · 2023-11-30T15:05:34Z

@abhijeetSaroha could you please create some tests? you can create a new makim file for test based on this other one: https://github.com/osl-incubator/makim/blob/main/tests/.makim-env.yaml

it could be tests/.makim-working-directory.yaml

some ideas for the tests (pseudo makim file, not the real one):

global:
- working directory: /tmp
- groups:
  group-setup:
    working directory: group1
    targets:
      target-setup:
        run: |
          # create here all the paths necessary for the test ex:
          mkdir -p /tmp/group1
  group1:
   targets:
    target-1:  # NO  working directory
       run: |
          import os
          assert os.getcwd() == "/tmp/group1"

    target-2 working directory: target2
      run: |
        import os
        assert os.getcwd() == "/tmp/group1/target2"

this is just an idea of example, try to create different kind of tests for the following states:

absolute path
relative path
no path

and you should combine this for all the 3 scopes example

global absolute path, group relative path, target no path
global absolute path, group relative path, target absolute path
global absolute path, group relative path, target relative path
global absolute path, group absolute path, target no path
global absolute path, group no path, target no path
...

maybe you will need to have 3 makim files for that because the 3 different states for the global path

tests/.makim-working-directory-absolute.yaml
tests/.makim-working-directory-relative.yaml
tests/.makim-working-directory-no-path.yaml

Create a new target here https://github.com/osl-incubator/makim/blob/main/.makim.yaml#L87
for the working-directory test, in the beginning of your test call the target for the setup, so it will create all the directory first

additionally, update this target in order to add your new targets for working directory smoke tests: https://github.com/osl-incubator/makim/blob/main/.makim.yaml#L69C7-L69C12

xmnlab · 2023-11-30T18:18:29Z

additionally, please create a new step in the CI workflow for testing the working directory feature. example: https://github.com/osl-incubator/makim/blob/main/.github/workflows/main.yaml#L86-L87

xmnlab · 2023-11-30T18:28:38Z

src/makim/makim.py

@@ -93,6 +95,16 @@ def _call_shell_app(self, cmd):
            _new_session=True,
        )

+        self.working_directory_scoped = None  # Reset scoped working directory


instead of setting working directory here,
please do it inside the change_target and change_group_data methods.
use the shell_app flow as a reference:

https://github.com/osl-incubator/makim/blob/main/src/makim/makim.py#L138

https://github.com/osl-incubator/makim/blob/main/src/makim/makim.py#L162-L163

https://github.com/osl-incubator/makim/blob/main/src/makim/makim.py#L169C1-L170

src/makim/makim.py

abhijeetSaroha · 2023-12-02T06:23:51Z

Update the code as we have discussed in that last coding session.

abhijeetSaroha · 2023-12-04T16:47:41Z

/tests/,makim-working-directory-absolute.yaml

version: 1.0
working-directory: "/tmp/tests"

groups:
  group-relative:
    working-directory: "group1"
    targets:
      target-no-path:
        help: Test global absolute path, group relative path, target no path
        run: |
          import os
          assert os.getcwd() == "/tmp/tests/group1"

      target-absoulte:
        working-directory: "/tmp"
        help: Test global absolute path, group relative path, target absolute path
        run: |
          import os
          assert os.getcwd() == "/tmp"

      target-realtive:
        working-directory: "target3"
        help: Test global absolute path, group relative path, target relative path
        run: |
          import os
          assert os.getcwd() == "/tmp/tests/group1/target3"

      target-no-path:
        help: Test global absolute path, group relative path, target no path
        run: |
          import os
          assert os.getcwd() == "/tmp/tests/group1"

  group-absolute:
    working-directory: "/tmp/tests/group2"
    targets:
      target-no-path:
        help: Test global absolute path, group absolute path, target no path
        run: |
          import os 
          assert os.getcwd() == "/tmp/tests/group2"

  group-no-path:
    targets:
      target-no-path:
        help: Test global absolute path, group no path, target no path
        run: |
          import os
          assert os.getcwd() == "/tmp/tests"

      target-absolute:
        working-directory: "/tmp"
        help: Test global absolute path, group no path, target absolute path
        run: |
          import os
          assert os.getcwd() == "/tmp"

      target-relative:
        working-directory: "group1/target3"
        help: est global absolute path, group no path, target relative path
        run: |
          import os
          assert os.getcwd() == "/tmp/tests/group1/target3"

Can you please review this, what are your thoughts about this?

I know run command is almost same for everyone, but I think for testing, it will be good.

Any other suggestion!

abhijeetSaroha · 2023-12-04T16:48:29Z

Mistakenly press Close with comment instead of Comment.

abhijeetSaroha · 2023-12-04T16:58:56Z

I decided to create three files based on these combinations.

tests/.makim-working-directory-absolute.yaml:

 * Global Absolute Path, Group Relative Path, Target No Path
 * Global Absolute Path, Group Relative Path, Target Absolute Path
 * Global Absolute Path, Group Relative Path, Target Relative Path
 * Global Absolute Path, Group Absolute Path, Target No Path
 * Global Absolute Path, Group No Path, Target No Path
 * Global Absolute Path, Group No Path, Target Absolute Path
 * Global Absolute Path, Group No Path, Target Relative Path

tests/.makim-working-directory-relative.yaml:

 * Global Relative Path, Group Relative Path, Target No Path
 * Global Relative Path, Group Relative Path, Target Absolute Path
 * Global Relative Path, Group Relative Path, Target Relative Path
 * Global Relative Path, Group Absolute Path, Target No Path
 * Global Relative Path, Group No Path, Target No Path
 * Global Relative Path, Group No Path, Target Absolute Path
 * Global Relative Path, Group No Path, Target Relative Path

tests/.makim-working-directory-no-path.yaml:

 * Global No Path, Group Relative Path, Target No Path
 * Global No Path, Group Relative Path, Target Absolute Path
 * Global No Path, Group Relative Path, Target Relative Path
 * Global No Path, Group Absolute Path, Target No Path
 * Global No Path, Group No Path, Target No Path
 * Global No Path, Group No Path, Target Absolute Path
 * Global No Path, Group No Path, Target Relative Path

xmnlab · 2023-12-04T17:23:03Z

@abhijeetSaroha you can have 3 tests for absolute path for group as well (instead of just one).

for example:

Global Absolute Path, Group Absolute Path, Target No Path (as you already mentioned)
Global Absolute Path, Group Absolute Path, absolute path
Global Absolute Path, Group Absolute Path, relative path

the same thing for the other configuration for global as well

…et and group

xmnlab · 2023-12-04T21:57:06Z

hey @abhijeetSaroha , it looks very nice, you are in right direction!
please add a step in the CI workflow for your new test, similar to this one: https://github.com/osl-incubator/makim/blob/main/.github/workflows/main.yaml#L86-L87

abhijeetSaroha · 2023-12-05T07:15:03Z

@xmnlab, It shows error for not finding the file / directory, which I have used and created for testing the working-directory-absolute-path. So, now I have to upload those directory also? or Is there any solution which should I approach or may I change the directory names in the testing file ?

xmnlab · 2023-12-05T10:49:39Z

@abhijeetSaroha as we discused before, we need a setup step before the tests. So maybe the easiest way for now, wpuld be create all the folders used by your tests in your in the .makim.yaml file inside the target you created at the beginning of the "run".

Use this example as reference: "mkdir -p /path/to/folder/"

Let me know if you need more explanation.

Thanks!!

abhijeetSaroha · 2023-12-05T12:28:31Z

@xmnlab , Now, It's creating problem different for macos and ubuntu. In macos testing, it shows that parent directory is /private but in linux, that's not the case.

xmnlab · 2023-12-05T13:04:49Z

@abhijeetSaroha , instead of using /private, use /tmp

the code for setup should go to a specific target, you can call that setup and call that as a dependency for each other target.

let me know if you need more details

xmnlab · 2023-12-05T18:04:17Z

you can have the setup in a group called setup and the target could be called folders

… file

…er issue

xmnlab · 2023-12-07T20:33:55Z

@abhijeetSaroha
now it is just missing some documentation
please create a new file at https://github.com/osl-incubator/makim/tree/main/docs
it could be called features.md let use that to describe all the new features

You can add the following information (just an idea of structure, feel free to improve that):

# Working Directory

attribute: working-directory
scopes: global, group, target

## Example

```yaml

version: 1.0
working-directory: /tmp/tests

groups:
  group1:
    working-directory: group1
    targets:
      target-no-path:
        help: example of the usage of working-directory attribute
        working-directory: target1
        run: |
          import os
          print(os.getcwd())


```

abhijeetSaroha · 2023-12-08T04:06:10Z

@xmnlab If you have time, please review the features.md file.

xmnlab · 2023-12-08T11:08:11Z

Looks really nice. We don't need the section "Benefits". Could you remove that please? Other than that, It looks great.

abhijeetSaroha · 2023-12-08T14:42:16Z

Removed the section of Benefit

github-actions · 2023-12-08T17:50:26Z

🎉 This PR is included in version 1.9.0 🎉

The release is available on:

1.9.0
GitHub release

Your semantic-release bot 📦🚀

add working-directory to the target, group and global scope

0ebcd53

xmnlab requested changes Nov 29, 2023

View reviewed changes

refractor working directory handling in _load_working_directory()

6960c39

xmnlab requested changes Nov 30, 2023

View reviewed changes

update the working-directory logic

f483001

abhijeetSaroha closed this Dec 4, 2023

abhijeetSaroha reopened this Dec 4, 2023

abhijeetSaroha added 2 commits December 5, 2023 00:17

add test cases for global absolute path and its combination with targ…

fc0d05e

…et and group

add new target for working-directory-absolute-path

f09a55d

add CI workflow for new test

d0a59e5

abhijeetSaroha added 4 commits December 5, 2023 17:13

update the setup for the test

ef68f60

finding issue for os.getcwd()

86f7362

solved issue for testing working-directory

32c1350

add /private to path of directories

7cc2e58

use /tmp instead of /private

792b7b9

abhijeetSaroha and others added 17 commits December 5, 2023 23:51

add the $ sign

7f33152

add the $ sign

c149132

.github/workflows/main.yaml

57dcefa

add test for global-no-path and its combination

c9d9ad9

solving issue for macos

eb3d4b4

resolve conflicts

6fd2515

resolve conflict

9743a3d

add working-directory-no-path

881bf43

add test file for global relative path and its combinations

5b8ad78

fix small issues and try to fix issues with macos

3044afa

rename test file

7bdb1db

rename test file

f524982

change sed to gsed on ci for macos

d17aa6d

on ci, replace /tmp to /private/tmp for macos for no-path makim file

dcd2d18

on ci, replace /tmp to /private/tmp for macos for relative-path makim…

58a26ba

… file

fix unrelated small issues in order to check if that can fix the lint…

d923252

…er issue

fix linter

9464298

xmnlab changed the title ~~WIP: add working-directory to the target, group and global scope~~ feat: Add working-directory to the target, group and global scope Dec 7, 2023

add new file features.md

5a4f500

fix docs

6814dc9

remove benefit section

4bd373d

xmnlab merged commit 3fbd61e into osl-incubator:main Dec 8, 2023
8 checks passed

github-actions bot added the released label Dec 8, 2023

xmnlab mentioned this pull request Dec 11, 2023

Add a parameter working-directory to global, group and target scope #48

Closed

abhijeetSaroha deleted the working-directory branch December 13, 2023 03:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add working-directory to the target, group and global scope #65

feat: Add working-directory to the target, group and global scope #65

abhijeetSaroha commented Nov 29, 2023 •

edited by xmnlab

Loading

abhijeetSaroha commented Nov 29, 2023

xmnlab left a comment

abhijeetSaroha commented Nov 30, 2023

xmnlab commented Nov 30, 2023 •

edited

Loading

xmnlab commented Nov 30, 2023

xmnlab Nov 30, 2023

abhijeetSaroha commented Dec 2, 2023

abhijeetSaroha commented Dec 4, 2023

abhijeetSaroha commented Dec 4, 2023

abhijeetSaroha commented Dec 4, 2023

xmnlab commented Dec 4, 2023

xmnlab commented Dec 4, 2023

abhijeetSaroha commented Dec 5, 2023 •

edited

Loading

xmnlab commented Dec 5, 2023

abhijeetSaroha commented Dec 5, 2023

xmnlab commented Dec 5, 2023

xmnlab commented Dec 5, 2023

xmnlab commented Dec 7, 2023

abhijeetSaroha commented Dec 8, 2023

xmnlab commented Dec 8, 2023

abhijeetSaroha commented Dec 8, 2023

github-actions bot commented Dec 8, 2023

feat: Add working-directory to the target, group and global scope #65

feat: Add working-directory to the target, group and global scope #65

Conversation

abhijeetSaroha commented Nov 29, 2023 • edited by xmnlab Loading

abhijeetSaroha commented Nov 29, 2023

xmnlab left a comment

Choose a reason for hiding this comment

abhijeetSaroha commented Nov 30, 2023

xmnlab commented Nov 30, 2023 • edited Loading

xmnlab commented Nov 30, 2023

xmnlab Nov 30, 2023

Choose a reason for hiding this comment

abhijeetSaroha commented Dec 2, 2023

abhijeetSaroha commented Dec 4, 2023

abhijeetSaroha commented Dec 4, 2023

abhijeetSaroha commented Dec 4, 2023

xmnlab commented Dec 4, 2023

xmnlab commented Dec 4, 2023

abhijeetSaroha commented Dec 5, 2023 • edited Loading

xmnlab commented Dec 5, 2023

abhijeetSaroha commented Dec 5, 2023

xmnlab commented Dec 5, 2023

xmnlab commented Dec 5, 2023

xmnlab commented Dec 7, 2023

abhijeetSaroha commented Dec 8, 2023

xmnlab commented Dec 8, 2023

abhijeetSaroha commented Dec 8, 2023

github-actions bot commented Dec 8, 2023

abhijeetSaroha commented Nov 29, 2023 •

edited by xmnlab

Loading

xmnlab commented Nov 30, 2023 •

edited

Loading

abhijeetSaroha commented Dec 5, 2023 •

edited

Loading