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

Enhancement/20 allow reading and writing shared config file #23

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

BoxG11
Copy link
Contributor

@BoxG11 BoxG11 commented May 21, 2022

All your requested changes have been made. Tests are all green

BoxG11 and others added 8 commits April 15, 2022 15:34
added a configuration loader which checks for existing config files.

If the folder is not found it is created.
If the file is not found the default empty config file is copied to users home directory.
added suggestions from pylint
Add a test case that loads a simple YAML file with two projects. You can
run tests by executing `pytest` in the project root. Note that the test
currently fails.
added yaml import
The tests now are green. I just changed it to return the contents of the yaml file.

In the test no path has to be passed. This is already done in the configloader. Can I remove this from the test @bkircher ?
Added misssing dependency pyyaml for pytest
The function was copying the file to the wrong location
@BoxG11 BoxG11 requested a review from bkircher May 21, 2022 14:06
@BoxG11 BoxG11 linked an issue May 21, 2022 that may be closed by this pull request
@bkircher
Copy link
Contributor

@jonathanlutterbeck Hey thanks! Can you

  • make sure that the linter passes in the CI run?
  • squash all commits into one

bkircher and others added 3 commits June 2, 2022 17:43
Checks have been added to verify file name lenght and to check if it's really a .yaml file.

All tests are now green.

Fixed error

Moved the syspath cariable to be globally available

Fix error

path variable was named wrong

Try to fix github action error

Changed file name for config file.

Fixed Test

Fixed copyfile for test case

Check if config is valid

load_config now gives correct error messages and does not catch any exceptions.

The functions are now also properly seperated.

removed print statements

Print statements have been removed and replaced them with RuntimeErrors where applicable.

which_path was renamed to default_config_path.

Improved code

I merged the two isInstance calls to improve readability.
@BoxG11 BoxG11 force-pushed the enhancement/20-allow-reading-and-writing-shared-config-file branch from e48f9ac to ebc5df8 Compare June 2, 2022 15:44
Jonathan, Lutterbeck added 2 commits June 2, 2022 18:15
commit ebc5df8
Author: Benjamin Kircher <[email protected]>
Date:   Wed May 4 10:05:50 2022 +0200

    tests: Ensure load_config does not interpret file name

commit 5fe3df0
Author: Jonathan Lutterbeck <[email protected]>
Date:   Mon Apr 25 19:45:13 2022 +0200

    improvements to check validity of config file

    Checks have been added to verify file name lenght and to check if it's really a .yaml file.

    All tests are now green.

    Fixed error

    Moved the syspath cariable to be globally available

    Fix error

    path variable was named wrong

    Try to fix github action error

    Changed file name for config file.

    Fixed Test

    Fixed copyfile for test case

    Check if config is valid

    load_config now gives correct error messages and does not catch any exceptions.

    The functions are now also properly seperated.

    removed print statements

    Print statements have been removed and replaced them with RuntimeErrors where applicable.

    which_path was renamed to default_config_path.

    Improved code

    I merged the two isInstance calls to improve readability.

commit a658543
Author: Benjamin Kircher <[email protected]>
Date:   Wed Apr 27 09:18:58 2022 +0200

    tests: Add more test cases for load_config()

commit 3566688
Merge: d7d1915 6aed011
Author: Jonathan Lutterbeck <[email protected]>
Date:   Mon Apr 25 19:42:41 2022 +0200

    Merge branch 'enhancement/20-allow-reading-and-writing-shared-config-file' of https://github.com/gridscale/gridscale_api_client_python into enhancement/20-allow-reading-and-writing-shared-config-file

commit d7d1915
Author: Jonathan Lutterbeck <[email protected]>
Date:   Mon Apr 25 19:41:45 2022 +0200

    Fixed error

    The function was copying the file to the wrong location

commit 6aed011
Author: jonathanlutterbeck <[email protected]>
Date:   Mon Apr 25 19:39:26 2022 +0200

    Changed yaml to pyyaml

    Added misssing dependency pyyaml for pytest

commit 415112c
Author: Jonathan Lutterbeck <[email protected]>
Date:   Mon Apr 25 19:15:34 2022 +0200

    Improve code to run checks

    The tests now are green. I just changed it to return the contents of the yaml file.

    In the test no path has to be passed. This is already done in the configloader. Can I remove this from the test @bkircher ?

commit 05c3775
Author: jonathanlutterbeck <[email protected]>
Date:   Fri Apr 22 11:27:59 2022 +0200

    Update dev-requirements.txt

    added yaml import

commit 2c62062
Author: Benjamin Kircher <[email protected]>
Date:   Fri Apr 22 11:12:10 2022 +0200

    Add test case for load_config

    Add a test case that loads a simple YAML file with two projects. You can
    run tests by executing `pytest` in the project root. Note that the test
    currently fails.

commit 09ef4af
Author: jonathanlutterbeck <[email protected]>
Date:   Fri Apr 15 20:34:34 2022 +0200

    improved code

    added suggestions from pylint

commit acb07f7
Author: jonathanlutterbeck <[email protected]>
Date:   Fri Apr 15 15:34:24 2022 +0200

    created configloader.py

    added a configuration loader which checks for existing config files.

    If the folder is not found it is created.
    If the file is not found the default empty config file is copied to users home directory.
api_config.api_key['X-Auth-UserId'] = "USER_UUID"
# api_config.debug = True

#TODO: Change filename

Choose a reason for hiding this comment

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

Why change the filename?

configfile = load_config("config.yaml")
api_config.api_key['X-Auth-Token'] = configfile[0].get("token")
api_config.api_key['X-Auth-UserId'] = configfile[0].get("userId")
api_config.debug = True

Choose a reason for hiding this comment

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

I think this shouldn't be debug per default

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.

Allow reading and writing shared config file
3 participants