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.
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
Blog: Connecting to a service without SBO #5915
Blog: Connecting to a service without SBO #5915
Changes from 4 commits
bc7c82f
a808ba2
78b7a37
62a7b46
e4cdcc4
46bb7b5
197905a
39c3211
bf76584
efd7338
013f6bc
d5715c8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find that exporting these variables at this point is premature. We cannot understand what they are for. I think it would be better to introduce the helm chart before to export them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, but then if we want to maintain the order of
odo init
>odo dev
> deploy charts > wait for odo dev to detect changes, it becomes necessary to export these variables at this pointThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The order
is just an example provided by Tomas, I'm not sure it must be respected at all price.
By first deploying the chart (including exportint variables), then running odo init and odo dev, the flow seems more understandable to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the current approach of enabling Helm repo first. But I don't agree with the idea of spinning up Helm chart using it before doing
odo init
. If it were me, I would first show that your Go application is deployed usingodo dev
, butcurl
request made to it is throwing 404 or some error. Thereafter, we spin up the MongoDB database required by the application.Or maybe, do an
odo dev
without all the variables which would help one connect to the MongoDB database. Make acurl
call that returns 404 or some error. Now add information about connection to MongoDB and see how the application returns expected output.I'm saying this because that's how I think an application developer's flow would be like. But then I have not a lot of application development myself, so correct me if I'm mistaken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you, but I can't find a flow that works for everyone. For now, I have added a note stating that
odo dev
will fail if run before adding connection information(bf76584).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me. I discussed this with @feloy and agree with the flow we have now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, I think it is premature to pass these variables, as they are not existing in the Devfile yet and we are not using them. I think it would be better to wait for steps 7.x before to do this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then I can simply not do
odo dev
first and put it after deploying the microservice.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could add to the conclusion that it is up to the reader to manage the secrets stored in the environment variables MY_MONGODB_ROOT_PASSWORD and MY_MONGODB_ROOT_USERNAME, so they are accessible every time the following command is executed:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added this note in the section where we introduce these variables for the first time, I think it makes more sense to put them there than in the conclusion.