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

More gracefully override JPS ExtensionApp.config_file_paths #612

Merged
merged 2 commits into from
Jul 12, 2024

Conversation

MetRonnie
Copy link
Member

@MetRonnie MetRonnie commented Jul 2, 2024

This allows patching in the tests so the tests don't pick up our user/site configs. Currently the tests fail locally for me because they pick up my config.

@MetRonnie MetRonnie changed the title Tests: auto-patch to avoid loading user/site configs Correctly override JPS ExtensionApp.config_file_paths Jul 2, 2024
@MetRonnie MetRonnie changed the title Correctly override JPS ExtensionApp.config_file_paths More gracefully override JPS ExtensionApp.config_file_paths Jul 2, 2024
@MetRonnie MetRonnie marked this pull request as ready for review July 2, 2024 17:49
@MetRonnie MetRonnie changed the base branch from master to 1.5.x July 2, 2024 17:49
@MetRonnie MetRonnie added this to the 1.5.1 milestone Jul 2, 2024
@MetRonnie MetRonnie added the small label Jul 2, 2024
Comment on lines +416 to +429
@property
def config_file_paths(self) -> List[str]:
if self._config_file_paths is None:
ret = get_conf_dir_hierarchy(
[
SITE_CONF_ROOT, # site configuration
USER_CONF_ROOT, # user configuration
], filename=False
)
# Next include currently needed for directory making
ret.insert(0, str(Path(uis_pkg).parent)) # packaged config
ret.reverse()
self._config_file_paths = ret
return self._config_file_paths
Copy link
Member

Choose a reason for hiding this comment

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

This change does not appear to be necessary, the only bit that's needed is the autouse?

Copy link
Member Author

@MetRonnie MetRonnie Jul 9, 2024

Choose a reason for hiding this comment

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

Couldn't get the monkeypatching to work without this change. Not sure why, but pyright is not happy with overriding a property with a class attribute anyway (see microsoft/pyright#3646 (comment))

Copy link
Member

Choose a reason for hiding this comment

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

Monkeypatching seems to work fine for me (with your config)?

I found the config_file_paths definition in Jupyter_Core and it is indeed a property so this makes sense 👍.

@oliver-sanders oliver-sanders merged commit 7a52238 into cylc:1.5.x Jul 12, 2024
2 of 4 checks passed
@MetRonnie MetRonnie deleted the fix-tests branch July 12, 2024 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants