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

refactor: move mode checks to getForwardablePorts #5022

Merged
merged 3 commits into from
Sep 5, 2023

Conversation

vvagaytsev
Copy link
Collaborator

What this PR does / why we need it:
Just a quick PR to minimize the changeset in #4846. That will make conflict resolution easier.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

@vvagaytsev vvagaytsev requested a review from a team September 5, 2023 09:03
Copy link
Contributor

@shumailxyz shumailxyz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@vvagaytsev vvagaytsev added this pull request to the merge queue Sep 5, 2023
Merged via the queue into main with commit e3b7545 Sep 5, 2023
41 checks passed
@vvagaytsev vvagaytsev deleted the refactor-get-forwardable-ports branch September 5, 2023 11:21
vvagaytsev added a commit that referenced this pull request Sep 7, 2023
Local mode checks were moved to `getForwardablePorts` in #5022.
vvagaytsev added a commit that referenced this pull request Sep 11, 2023
Local mode checks were moved to `getForwardablePorts` in #5022.
vvagaytsev added a commit that referenced this pull request Sep 12, 2023
Local mode checks were moved to `getForwardablePorts` in #5022.
vvagaytsev added a commit that referenced this pull request Sep 12, 2023
Local mode checks were moved to `getForwardablePorts` in #5022.
vvagaytsev added a commit that referenced this pull request Sep 13, 2023
Local mode checks were moved to `getForwardablePorts` in #5022.
vvagaytsev added a commit that referenced this pull request Sep 20, 2023
Local mode checks were moved to `getForwardablePorts` in #5022.
github-merge-queue bot pushed a commit that referenced this pull request Sep 21, 2023
* refactor: extract named function for manifest post-processing

* refactor: split manifest reading and post-processing

* fix(k8s): correctly resolve manifests when `build` is set

* fix(k8s): quickfix to avoid resetting `outdated` state to `ready`

* test: fix tests

* refactor(test): move helper to the upper-level scope

It will be used by other test contexts.

* refactor(test): introduce one more convenience helper

* refactor(test): introduce and rename some local vars

* refactor(test): use helpers to avoid code duplication

* chore: re-arranged code

Keep all helpers close to each other for simpler navigation.

* chore(test): rename some tests and variables

To avoid usage of the old glossary.

* refactor(test): helper function to deploy in namespaces

* DRY to avoid repetition and too complex local state
* Less shared global state between different test contexts
* Avoid dependencies between tests and reliance on the execution order
* Ability to run individual tests locally

* refactor(test): convert lambdas to functions

* test: ensure all resources are deleted by `deleteKubernetesDeploy`

A metadata `ConfigMap` describing what was last deployed must be deleted too.

* test: restore initial module config state after each test

To avoid unexpected pollution of the Garden instance's state.
Multiple test can define temporary custom module config.
Such config changes should not affect the other tests.

* refactor(k8s): helper to compose k8s deploy status

* refactor(k8s): return deploy status immediately on "missing" state

* refactor: unwrap unnecessary else-block

* chore: variable scoping

* refactor: extract helper to check if deploy is outdated

* refactor: move input args check into `getForwardablePorts`

* chore: remove unnecessary code and comments

Local mode checks were moved to `getForwardablePorts` in #5022.

* chore: remove unnecessary try/catch

Function `getForwardablePorts` does not call any potentially unsafe operations.
It's not wrapped into try/catch in the other code places.

* chore: remove unused local var

* refactor: use SRP in status calculation

Split individual resource status retrieval and overall state calculation.

* refactor: simplified code

Minimized mutability of the local vars.
More straightforward and linear processing flow.

* test: fix test setup to cover the original bug

* chore: post-merge corrections

* refactor: remove unnecessary function call

The value has already been calculated as an immutable value.

* chore: typos and wording

Got rid of old-fashioned term "service". Replaced it with "deploy".

---------

Co-authored-by: Jon Edvald <edvald@gmail.com>
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.

None yet

2 participants