8000 allow to get dependency builds env vars by jLopezbarb · Pull Request #4716 · okteto/okteto · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

allow to get dependency builds env vars #4716

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
May 23, 2025
Merged

allow to get dependency builds env vars #4716

merged 6 commits into from
May 23, 2025

Conversation

jLopezbarb
Copy link
Contributor

Signed-off-by: Javier Lopez javier@okteto.com

Proposed changes

Fixes DEV-940

When building and using smart build store the build env vars in the configmap so it can be used by any dependant service. The name of the variable is fmt.Sprintf("OKTETO_DEPENDENCY_%s_BUILD_%s_%s", dependencyName, build, buildEnvVar)

How to validate

  1. Check the following repos:
  1. Using this CLI as the CLI for the backend deploy the https://github.com/jLopezbarb/hello-world-chart and check that is working correctly

CLI Quality Reminders 🔧

For both authors and reviewers:

  • Scrutinize for potential regressions
  • Ensure key automated tests are in place
  • Build the CLI and test using the validation steps
  • Assess Developer Experience impact (log messages, performances, etc)
  • If too broad, consider breaking into smaller PRs
  • Adhere to our code style and code review guidelines

@jLopezbarb
allow to get dependency builds env vars
8000
3191b30 
Signed-off-by: Javier Lopez <javier@okteto.com>
@jLopezbarb jLopezbarb requested a review from a team as a code owner May 13, 2025 16:54
@jLopezbarb
fix: update function comment for GetConfigmapBuildEnvVars to reflect …
81492bd 
…correct data retrieval

Signed-off-by: Javier Lopez <javier@okteto.com>
@jLopezbarb
fix: initialize CurrentStore in TestBuildImages for proper context setup
5392864 
Signed-off-by: Javier Lopez <javier@okteto.com>
@codecov Codecov
Copy link
codecov bot commented May 14, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 46.57534% with 39 lines in your changes missing coverage. Please review.

Project coverage is 48.78%. Comparing base (46434c9) to head (1c062fc).
Report is 1 commits behind head on master.

❌ Your patch status has failed because the patch coverage (46.57%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #4716      +/-   ##
==========================================
- Coverage   48.79%   48.78%   -0.01%     
==========================================
  Files         354      354              
  Lines       29567    29638      +71     
==========================================
+ Hits        14428    14460      +32     
- Misses      13989    14023      +34     
- Partials     1150     1155       +5
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ifbyol
ifbyol reviewed May 19, 2025
View reviewed changes
Copy link
Member
@ifbyol ifbyol left a comment

Choose a reason for hiding this comment

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

I would consider to add an e2e for this case, as this might be a tricky case, but it has to be done once the CLI is integrated in the backend

jLopezbarb added 2 commits May 19, 2025 16:56
@jLopezbarb
fix: handle missing build-envs in GetConfigmapBuildEnvVars function
ac9cc50 
Signed-off-by: Javier Lopez <javier@okteto.com>
@jLopezbarb
fix: update build-envs to buildEnvs for consistency in code and tests
236e15d 
Signed-off-by: Javier Lopez <javier@okteto.com>
@jLopezbarb
Copy link
Contributor Author

I would consider to add an e2e for this case, as this might be a tricky case, but it has to be done once the CLI is integrated in the backend

Created a ticket for the next sprint

@jLopezbarb jLopezbarb merged commit 39c7040 into master May 23, 2025
12 of 13 checks passed
@jLopezbarb jLopezbarb deleted the jlo/dev-940 branch May 23, 2025 12:36
@BrewTestBot BrewTestBot mentioned this pull request Jun 11, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ifbyol ifbyol ifbyol approved these changes

Assignees
No one assigned
Labels
release/new-feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jLopezbarb @ifbyol
0