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

ENDOC-628 generate MS and MFE Tutorial edit commands #652

Merged
merged 3 commits into from
Dec 20, 2022
Merged

Conversation

jyunmitch
Copy link
Collaborator

No description provided.

```
>Note: On some Zsh interfaces, you may receive an invalid argument response that can be ignored as the long as the folders are relocated correctly.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not really happy with this, unsure how to put it

Copy link
Member

Choose a reason for hiding this comment

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

-> "In some shells (e.g. zsh), you may receive an "invalid argument response" warning that can be ignored as long as the folders are relocated correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

no comment per pending change to nathan's wording

```
>Note: On some Zsh interfaces, you may receive an invalid argument response that can be ignored as the long as the folders are relocated correctly.
Copy link
Contributor

Choose a reason for hiding this comment

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

no comment per pending change to nathan's wording

ent bundle api add conference-details conference-api --serviceName=conference-ms --serviceUrl=http://localhost:8081
ent bundle api add conference-form conference-api --serviceName=conference-ms --serviceUrl=http://localhost:8081
```
5. For local development and testing, a custom command is needed in the `entando.json` to use a different port for each MFE, shown in the following:
Copy link
Contributor

Choose a reason for hiding this comment

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

...the entando.json requires a custom command to...

as shown here/below

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried the first item with "entando.json requires", but it doesn't read right because it is not the entando.json that requires it

Copy link
Contributor

Choose a reason for hiding this comment

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

"...the entando.json must include..."

@@ -22,7 +22,7 @@ ent bundle svc start keycloak
``` sh
ent bundle run conference-ms
```
To check that the MS is working, go to `http://localhost:8081/`. In local development, the run command can be used to modify the port to run multiple microservices, but in production, microservices must run on port 8081.
To check that the MS is working, go to `http://localhost:8081/`. In local development, the run command can be used to modify the ports to run multiple microservices, but in production, microservices must run on port 8081.
Copy link
Contributor

Choose a reason for hiding this comment

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

"modify the ports to run multiple MSs" still reads that a single port can run multiple MSs. i think we don't need to add extra details or motivations here, just "to modify the port, but in..."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

reworded. "modify the port, but in production..." doesn't provide enough info

c. Add the API claims
```shell
ent bundle api add conference-details conference-api --serviceName=conference-ms --serviceUrl=http://localhost:8081
ent bundle api add conference-form conference-api --serviceName=conference-ms --serviceUrl=http://localhost:8081
Copy link
Contributor

Choose a reason for hiding this comment

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

a/b/c here don't end with : but the other instructions do

@@ -62,7 +62,7 @@ ent bundle run simple-node-ms
* Access <http://localhost:8081/api/hello> to see a `Hello World!` response with a timestamp.
* Access <http://localhost:8081/api/health> to see a standard `{"status:"UP"}` response.

In local development, the run command can be used to modify the port to run multiple microservices, but in production, microservices must run on port 8081.
In local development, custom commands can be used to modify ports to run each microservice, but in production, microservices must run on port 8081.
Copy link
Contributor

Choose a reason for hiding this comment

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

i think this is less clarified... prefer nathan's tweak to your original wording: In local development, the run command can be used to modify the port for a microservice, but in production microservices must run on port 8081.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted to include cases where there are multiple microservices because that's when this applies, not when there is just one

Copy link
Contributor

Choose a reason for hiding this comment

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

In local development, the run command can be used to modify the port number so that multiple microservices can run in parallel on separate ports, but in production microservices must run on port 8081

@jyunmitch jyunmitch merged commit 3b91b37 into main Dec 20, 2022
@jyunmitch jyunmitch deleted the ENDOC-628 branch December 20, 2022 20:52
@jyunmitch jyunmitch restored the ENDOC-628 branch December 21, 2022 15:51
@jyunmitch jyunmitch deleted the ENDOC-628 branch December 21, 2022 15:53
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.

3 participants