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

[#584] load settings from environment without a config file present #585

Merged
merged 1 commit into from
Aug 3, 2024

Conversation

d-w-moore
Copy link
Collaborator

@d-w-moore d-w-moore commented Jul 12, 2024

See issue description. Basically this change means that you won't have to declare a config file path in the environment (even if that value is null length, denoting the default path of ~/.python_irodsclient) in order to get the environmental overrides to work.

@d-w-moore d-w-moore changed the title [#584] load settings from environment even without a config file present [#584] load settings from environment without a config file present Jul 12, 2024
Copy link
Contributor

@alanking alanking left a comment

Choose a reason for hiding this comment

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

Seems fine. Does it make sense to include a test for this?

@d-w-moore
Copy link
Collaborator Author

Seems fine. Does it make sense to include a test for this?

It does indeed.

Copy link
Contributor

@alanking alanking left a comment

Choose a reason for hiding this comment

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

Test looks good to me. Had a couple of fortification suggestions and then I think this should be good to go.

irods/test/data_obj_test.py Outdated Show resolved Hide resolved
irods/test/data_obj_test.py Show resolved Hide resolved
irods/test/modules/test_xml_parser.py Show resolved Hide resolved
Copy link
Contributor

@alanking alanking left a comment

Choose a reason for hiding this comment

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

Just one last thing about that skip and I'm happy with it

irods/test/data_obj_test.py Show resolved Hide resolved
irods/test/data_obj_test.py Show resolved Hide resolved
@d-w-moore
Copy link
Collaborator Author

d-w-moore commented Aug 2, 2024

Responding to #585 (comment) @alanking 's comment. above (Sorry I cannot respond int eh conversation above, but I guess GitHub works in strange ways.)

The os.path.absdir may not be needed, as :

  1. it would only be needed for Python2, for which we soon plan to drop support
  2. the only eventuality that would cause the join's result to be inaccurate in finding the target script is an invocation of os.chdir by another test, and this will not happen, since setupssl.py is the only thing calling chdir, and its only caller (login_auth_test.py) has now been banished from the test suite.

Copy link
Contributor

@alanking alanking left a comment

Choose a reason for hiding this comment

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

Alright, let's get this squashed, @d-w-moore. Omit #s for now.

@d-w-moore
Copy link
Collaborator Author

Squashed!

Copy link
Contributor

@alanking alanking left a comment

Choose a reason for hiding this comment

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

# it

@d-w-moore
Copy link
Collaborator Author

Done.

@alanking alanking merged commit bf46679 into irods:main Aug 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants