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

fix: include load with relative root path #1049

Merged
merged 2 commits into from
Mar 1, 2024

Conversation

pedro-psb
Copy link
Member

The include loader was not handling the relative root_path correctly.

Closes #1025

Copy link

netlify bot commented Feb 10, 2024

Deploy Preview for dynaconf ready!

Name Link
🔨 Latest commit 2320321
🔍 Latest deploy log https://app.netlify.com/sites/dynaconf/deploys/65e1dcfda59f050008999527
😎 Deploy Preview https://deploy-preview-1049--dynaconf.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Feb 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.80%. Comparing base (6daf249) to head (2320321).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1049   +/-   ##
=======================================
  Coverage   98.80%   98.80%           
=======================================
  Files          23       23           
  Lines        2257     2257           
=======================================
  Hits         2230     2230           
  Misses         27       27           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

p
for p in sorted(glob(filepath, root_dir=self._root_path))
if ".local." not in p
p for p in sorted(glob(filepath)) if ".local." not in p
Copy link
Member Author

Choose a reason for hiding this comment

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

filepath here is assumed to contain the root_path already (see lines right above), so passing the root_path again to glob duplicates it in the final path (e.g etc/etc/filename.toml) making it unable to find the file.

tox.ini Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to be able to pass pytest args to tox. It was useful in the PR, because it was python-version dependent (only python>=3.10 has the root_dir glob parameter).

@rochacbruno rochacbruno added this to the 3.2.5 milestone Mar 1, 2024
@pedro-psb pedro-psb merged commit fa8c748 into dynaconf:master Mar 1, 2024
44 checks passed
@pedro-psb pedro-psb deleted the fix-include-with-root-path branch March 1, 2024 14:33
pedro-psb added a commit that referenced this pull request Mar 18, 2024
Shortlog of commits since last release:

    Aaron DeVore (1):
          docs: fix incorrect combination of TOML table and inline table (#1070)

    Adam Kjems (1):
          doc: replace dead link to flask subclassing page (#1031)

    Bruno Rocha (2):
          feat: Add `@get` converter to alias existing keys (#1040)
          fix: dependabot alert 21 about Django (on tests) (#1067)

    HAMASHITA (1):
          chore: Fix misspelled variable name (#1032)

    Lucas Limeira (1):
          doc: Add explicit Dynaconf instantiation to sample code (#1022)

    Mitchell Edmunds (5):
          docs: Add dynaconf API to docs with mkdocstrings (#1058)
          docs: Fix mkdocs warnings for cleaner build output (#1061)
          chore: add "typos" tool and run it in codebase/docs (#1063)
          Fix referencing issues in the docs (#1062)
          chore(ci): Replace lint and formatting tools with ruff (#1074)

    Mostafa Alayesh (1):
          doc: fix argument `env` in Validation at validation.md (#1051)

    Pedro Brochado (11):
          chore(ci): Fix misspelled GitHub action names on main.yml (#1033)
          chore(ci): move release workflow to GitHub actions (partial) (#1043)
          chore(ci): fix shell script in release.yml
          chore(ci): fix shell script in release.yml (2)
          chore(ci): fix create-release-commit.sh permission
          fix: include load with relative root path (#1049)
          fix: `_bypass_evaluation` showing in end-user settings (#1071)
          chore: Replace/Update release script (#1078)
          docs: fix wrong info about validation trigger on insantiation (#1076)
          misc: fix bump-my-version invalid config and rename bump msg
          misc: fix changelog generation order

    Shanshi Shi (1):
          Fix a typo in the docs about `merge_enabled` setting (#1044)

    Sun Jianjiao (1):
          doc: Fix the syntax errors in the sample program. (#1027)

    tdzz1102 (1):
          [Doc] Fix a small mistake of .env file in the document (#1036)

    xiaohuanshu (1):
          doc: fix click help syntax error (#1041)
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.

[bug] dynaconf 3.2.2 broke dynaconf_include when combined with root_path (still broken on 3.2.4)
2 participants