Make default Config() Initialization respect target_path in default config file

[ryanquincypaul]

The problem

In databricks python tasks using serverless compute, the default current working directory doesn't allow writes. The logo and version checking code instantiates a Config object with the default values and no way to configure them which means target_path goes to DEFAULT_TARGET_PATH = os.getcwd() + "/edr_target" and no way to override it. Below the solution I will put lines of code of the issues in case that is relevant.

What I would like to have happen

I don't think the logo and version checking logic should require file writes that I have no option of skipping at all, but in lieu of that, instead of os.chdir()-ing the working directory to somewhere like /tmp, I'd rather just use the already existing target_path option to tell edr to put files in a place where I am allowed to write. The logo and upgrade check calls use a default Config() option which forces a default path using os.getcwd(). I would like it so at least options in a config file written at the default location (~/.edr/config.yml) is respected in a default Config() call. I can then at least give it a target-path: /tmp/WHATEVER_{unique_timestamp} or something so it can create the edr_target directory just to print the logo and check the dbt and python package version numbers in peace. It would be great if it would also respect the arguments I am passing in as I am using the --target-path parameter for edr report to store it somewhere in a databricks volume.

What I did

I am just setting the default value of target_path to None so that it gets to the check for a value in the default config directory in the _first_not_none set, and then it will go to the original DEFAULT_TARGET_PATH. I am also setting the DBT_LOG_PATH env var to the target_dir and not the passed in target_path.

• New default:

https://github.com/ryanquincypaul/elementary/blob/8243a7068b21a72ad0bbc921b5f04c0bca6131b3/elementary/config/config.py#L49

• New value checking and DBT_LOG_PATH setting code:

https://github.com/ryanquincypaul/elementary/blob/8243a7068b21a72ad0bbc921b5f04c0bca6131b3/elementary/config/config.py#L93-L100

Other solutions I had thought of

• Make sure logo and version checking run after arguments have been parsed so they can be used OR after a Config object has already been built in a way that respects cli arguments
  • This would still require changes to the config code because target_path argument is used as is for the purposes of setting DBT_LOG_PATH
  • Also that would defeat the purpose of these being simple things that always run at the top regardless if they require file writes lol.
• Just add more environment variables that can be used to skip the whole logo and version calls OR to manually override that DEFAULT_TARGET_PATH in Config class
  • A little too nuclear and hacky. Also I don't see a wide usage of environment variable overrides in the code base so not really fitting the paradigm.

More details on issue

Should be noted that there were PRs made and accepted adding configuration to disable the logo and version checking:

#1903
#1674

• Here are calls to print the logo and do the dbt vs python package version checking at the top of the cli file.

elementary/elementary/cli/cli.py

Lines 15 to 16 in 398b34a

	elementary.cli.logo.print_elementary_logo()
	elementary.cli.upgrade.recommend_version_upgrade()

• There are PRs that added options skip them but because they both use Config() with no parameters given or any other way to override the target_path.

elementary/elementary/cli/logo.py

Lines 15 to 21 in 398b34a

	def print_elementary_logo():
	config = Config()
	if config.disable_elementary_logo_print:
	return
	else:
	click.echo(f"{ELEMENTARY_LOGO}\n")

elementary/elementary/cli/upgrade.py

Lines 10 to 14 in 398b34a

	def recommend_version_upgrade():
	config = Config()
	if config.disable_elementary_version_check:
	return

• In the Config code, we can see the following default behavior:
  • Defaults for config and target_path

elementary/elementary/config/config.py

Lines 36 to 38 in 398b34a

	DEFAULT_CONFIG_DIR = str(Path.home() / ".edr")
	DEFAULT_TARGET_PATH = os.getcwd() + "/edr_target"

• Default arguments

elementary/elementary/config/config.py

Line 44 in 398b34a

config_dir: str = DEFAULT_CONFIG_DIR,

elementary/elementary/config/config.py

Line 49 in 398b34a

target_path: str = DEFAULT_TARGET_PATH,

• The check for target_path that has no way of getting to the config file since the default will always be present and I'm not sure you can even pass None via the command line if these default Config() calls even listened to that input. Furthermore, the DBT_LOG_PATH setting doesn't even respect the config path anyway, it's only using what's passed into the target_path parameter which again is just the default one in logo and variable checking code.

elementary/elementary/config/config.py

Lines 93 to 99 in 398b34a

	self.target_dir = self._first_not_none(
	target_path,
	config.get("target-path"),
	os.getcwd(),
	)
	os.makedirs(os.path.abspath(self.target_dir), exist_ok=True)
	os.environ["DBT_LOG_PATH"] = os.path.abspath(target_path)

Summary by CodeRabbit

• Refactor
  • Made the target path setting optional and adjusted the fallback to use the default target directory when unset.
  • Logging now points to the resolved target directory rather than the raw input.
  • Directory creation behavior remains based on the resolved target directory.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Made Config.target_path optional (default None). Resolve self.target_dir via first_not_none(target_path, config.get("target-path"), self.DEFAULT_TARGET_PATH). Use os.path.abspath(self.target_dir) for directory creation and DBT_LOG_PATH. Removed fallback to os.getcwd().

Changes

Cohort / File(s)	Summary
Config class changes elementary/config/config.py	Config.__init__ changed target_path: str = DEFAULT_TARGET_PATH → target_path: Optional[str] = None. Compute self.target_dir with first_not_none(target_path, config.get("target-path"), self.DEFAULT_TARGET_PATH). Use os.path.abspath(self.target_dir) for directory creation and DBT_LOG_PATH. Removed fallback to os.getcwd().

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I nudged a path from must to maybe,
Now hop to Default when none come lately,
Absolute paws steady the trail,
Logs follow close without fail,
A tiny change—big calm in the dale.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: making Config() initialization respect the target_path parameter from the default config file instead of defaulting to os.getcwd().
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

👋 @ryanquincypaul
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in this pull request.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@elementary/config/config.py`:
- Around line 93-101: Remove the transient stdout debug prints that can leak
secrets: eliminate the print(...) calls after computing self.target_dir in the
constructor where self.target_dir = self._first_not_none(target_path,
config.get("target-path"), self.DEFAULT_TARGET_PATH) is set; if you need
diagnostics, replace those prints with a call to a logger (e.g. logger.debug)
that redacts sensitive fields from config (or only logs non-sensitive fields
like the resolved DEFAULT_TARGET_PATH and final target_dir) and ensure the code
references the same symbols (_first_not_none, target_path, config,
DEFAULT_TARGET_PATH, target_dir) so the behavior and debugging remain clear
without printing secrets to stdout.

[ryanquincypaul]

I did not mean to make this to the main repo, just my fork as I was having trouble with local unit tests even with unmodified code. I was just pushing to see the unit tests run in the github actions as I couldn't get them to pass locally. I was just going to build a wheel from this and use it manually in databricks tasks! But whatever it is out there now. I will make a little more effort in the description.

[ryanquincypaul]

To be clear, i'm not requesting anything. It would be cool if this was builtin, but as of now I am just going to build a wheel and try it out in databricks now and I can report back. I didn't mean for this to be visible lol. Open to any feedback though regardless.

[ryanquincypaul]

@coderabbitai summary

[coderabbitai]

✅ Actions performed

Summary regeneration triggered.

[ryanquincypaul]

So this worked! But adding target-path: /tmp/whatever and staying in the default current working directory and providing the actual directory path for the profiles-dir and project-dir arguments still didn't work because it couldn't write a package-lock.json file when running the actual command. So I end up just changing the directory anyway. I'll keep this open as I continue to work on this and set a reminder to check in on friday and close this if there is no change.