Skip to content

issue-1866#1867

Closed
klyusba wants to merge 4 commits into
astronomer:mainfrom
klyusba:issue-1866
Closed

issue-1866#1867
klyusba wants to merge 4 commits into
astronomer:mainfrom
klyusba:issue-1866

Conversation

@klyusba
Copy link
Copy Markdown

@klyusba klyusba commented Jul 10, 2025

Description

fix ProfileConfig.get_profile_type()

Related Issue(s)

closes #1866

@netlify
Copy link
Copy Markdown

netlify Bot commented Jul 10, 2025

Deploy Preview for sunny-pastelito-5ecb04 canceled.

Name Link
🔨 Latest commit 10e5354
🔍 Latest deploy log https://app.netlify.com/projects/sunny-pastelito-5ecb04/deploys/6893280d64a1090008d9ecf4

Copy link
Copy Markdown
Collaborator

@tatiana tatiana left a comment

Choose a reason for hiding this comment

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

@klyusba thanks a lot for reporting the bug and submitting a fix for it!

The approach makes sense - and I am looping in @pankajastro to help reviewing, since he worked on this part of the code more recently.

Some requests:

  1. Please, could you include a test case that validates the error you reported in #1866 in a way it only works after your change?
  2. Could you also add a more meaningful PR title, similar to #1866?

Comment thread cosmos/config.py
with open(self.profiles_yml_filepath) as file:
profiles = yaml.safe_load(file)

profile_path = self._get_profile_path()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

By removing this, we are not using the profile caching introduced in #1046 - but this is likely unecessary when checking the profile type. @pankajastro could you share your thoughts on this one, please?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you please test this on Cosmos 1.12.0? I believe PR #2198 should have fixed the issue.

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 6, 2025

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.06%. Comparing base (19861f6) to head (10e5354).
⚠️ Report is 47 commits behind head on main.

Files with missing lines Patch % Lines
cosmos/config.py 0.00% 6 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1867   +/-   ##
=======================================
  Coverage   98.06%   98.06%           
=======================================
  Files          85       85           
  Lines        5313     5313           
=======================================
  Hits         5210     5210           
  Misses        103      103           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Sep 5, 2025

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed and removed stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed labels Sep 5, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 6, 2025

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed and removed stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed labels Oct 6, 2025
@pankajastro
Copy link
Copy Markdown
Contributor

I believe PR #2198 should have fixed the issue. Please feel free to re-open or create a new PR if you still encounter this issue.

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] ProfileConfig.get_profile_type fails with profiles_yml_filepath being set

4 participants