odo dev store information about currently forwarded ports #5703
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for odo-docusaurus-preview canceled.
|
openshift-ci
bot
added
the
kind/feature
feloy
force-pushed
the
feature-5676/odo-state-file
branch
2 times, most recently
from
949f06f
to
fce2b60
Compare
feloy
force-pushed
the
feature-5676/odo-state-file
branch
12 times, most recently
from
882b584
to
c1523ef
Compare
rm3l
requested changes
May 2, 2022
docs/website/versioned_docs/version-3.0.0/command-reference/dev.md
Outdated
Show resolved
Hide resolved
rm3l
requested changes
May 3, 2022
feloy
force-pushed
the
feature-5676/odo-state-file
branch
3 times, most recently
from
9a4d0a9
to
816ddb3
Compare
rm3l
requested changes
May 3, 2022
docs/website/versioned_docs/version-3.0.0/command-reference/dev.md
Outdated
Show resolved
Hide resolved
feloy
force-pushed
the
feature-5676/odo-state-file
branch
2 times, most recently
from
f50bff5
to
fc6d6b9
Compare
rm3l
requested changes
May 4, 2022
docs/website/versioned_docs/version-3.0.0/command-reference/dev.md
Outdated
Show resolved
Hide resolved
rm3l
approved these changes
May 4, 2022
openshift-ci
bot
added
the
lgtm
…v.md Co-authored-by: Armel Soro <armel@rm3l.org>
In case of error during initialization, odo dev cleanup the resources
feloy
force-pushed
the
feature-5676/odo-state-file
branch
from
02b56ba
to
6382aba
Compare
openshift-ci
bot
removed
the
lgtm
|
Kudos, SonarCloud Quality Gate passed!
|
dharmit
reviewed
May 5, 2022
Comment on lines
+6
to
+7
| // SetForwardedPorts sets the forwarded ports in the state file and saves it to the file, updating the metadata | ||
| SetForwardedPorts(fwPorts []api.ForwardedPort) error |
There was a problem hiding this comment.
Thinking out loud - why do we want business layer to get involved with storing the state? Isn't this something that odo "CLI" is interested in doing? Maybe a different implementation using odo as library would not want to store the state.
There was a problem hiding this comment.
In any case, we want to create an abstraction for the state, hence this package.
For me, it is a business concern, as it is a way to be able to give to the CLI the state of the Dev session at any time. Any other CLI would be interested in getting this state.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dharmit The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/lgtm |
openshift-ci
bot
added
the
lgtm
cdrage
pushed a commit
to cdrage/odo
that referenced
this pull request
Aug 31, 2022
…eloper#5703) * Save state (forwarded ports + PID + timestamp) to local file and cleanup * Integration tests * Documentation * Update docs/website/versioned_docs/version-3.0.0/command-reference/dev.md Co-authored-by: Armel Soro <armel@rm3l.org> * Fail when an error happens writing state file In case of error during initialization, odo dev cleanup the resources * Fix typo * Test localPort matches a number * Remove PID and timestamp from state + rename to devstate.json * Run IBM cloud tests as non root * Fix doc * Review * Remove reference to PID/timestamp from doc Co-authored-by: Armel Soro <armel@rm3l.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this:
/kind feature
What does this PR do / why we need it:
See issue.
The PR updates the image used to run tests on IBM Cloud to be able to run tests as non-root user, to make a new test relying on user rights pass.
Which issue(s) this PR fixes:
Fixes #5676
PR acceptance criteria:
Unit test
Integration test
Documentation
How to test changes / Special notes to the reviewer:
The values for the forwarded ports are obtained through the
PortWriter, from the text written by the low level port forwarder. This is because we need to get the information of the local port when we are using random ports (with --random-ports flag).