Feature/postgesql fix #2
Merged
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
New AWS sub-generator which uses Amazon ECS to run JHipster monolithic applications. Closes jhipster#6773
Moved constants from index.js into a separate aws-containers generator specific file
# Conflicts: # generators/aws-containers/index.js # generators/aws-containers/prompts.js # generators/aws-containers/templates/_application.template.yml # generators/aws-containers/templates/_base.template.yml # generators/docker-base.js # generators/docker-utils.js # generators/generator-base.js
carvallegro
reviewed
Feb 26, 2018
generators/aws-containers/prompts.js
Outdated
| const prodDatabaseType = config.prodDatabaseType; | ||
|
|
||
| if(prodDatabaseType === databaseTypes.postgresql) { | ||
| this.log.ok(` ⚠️ Postgresql databases are currently only supported by Aurora on high-performance database instances`); |
There was a problem hiding this comment.
This is not ok, The hot tub is too hot.
change the log and use chalk to either use orange or red
generators/aws-containers/prompts.js
Outdated
| }; | ||
| }); | ||
|
|
||
| if (perf.database.supportedEngines.includes(prodDatabaseType)) { |
There was a problem hiding this comment.
what is the else return value?
There was a problem hiding this comment.
Also, could you put the check perf.database.supportedEngines.includes(prodDatabaseType) in a variable?
I'm thinking isEngineSupported
| choices: performanceLevels, | ||
| default: awsConfig.performance, | ||
| validate: (input) => { |
There was a problem hiding this comment.
Why did you return the validation?
There was a problem hiding this comment.
Because it doesn't work. 🌮 🌷
| @@ -1,3 +1,4 @@ | |||
|
|
|||
generators/aws-containers/prompts.js
Outdated
|
|
||
| return 'Stack\'s name cannot be empty!'; | ||
| } | ||
| validate: input => ((_.isEmpty(input) || !input.match(CLOUDFORMATION_STACK_NAME)) ? 'Stack name must contain letters, digits, or hyphens ' : true) |
There was a problem hiding this comment.
You can ditch the parenthesis. Also I don't know how I feel about the ternary operator for this
generators/aws-containers/prompts.js
Outdated
|
|
||
| const AURORA_DB_PASSORD_REGEX = /^[^@"\/]{8,42}$/; // eslint-disable-line | ||
| const CLOUDFORMATION_STACK_NAME = /[a-zA-Z][-a-zA-Z0-9]*/; // eslint-disable-line | ||
|
|
||
| const SCALING_TO_CONFIG = { |
There was a problem hiding this comment.
I think you should merge back the two objects and then move the map done in both scaling and performance prompts here as constants.
Example: performanceLevels from promptPerformance becomes PERFORMANCE_PROMPT_CHOICES.
There was a problem hiding this comment.
Just realised that we did those mapping in the function when there is no need to do so as we don't need anything from the running context
There was a problem hiding this comment.
I'd prefer to leave this because I've separated the concepts.
| let dependency = `${group}:${name}`; | ||
| if (version) { | ||
| dependency += `:${version}`; | ||
| } | ||
| try { | ||
| jhipsterUtils.rewriteFile({ | ||
| file: fullPath, | ||
| path: directory, | ||
| file: 'build.gradle', |
BenEdridge
pushed a commit
that referenced
this pull request
Feb 27, 2018
* new aws-containers sub-generator New AWS sub-generator which uses Amazon ECS to run JHipster monolithic applications. Closes jhipster#6773 * external constants to localised file Moved constants from index.js into a separate aws-containers generator specific file * testing and linting errors * add back missing constants from aws-containers * address issue where the base.template is not being read correctly * Pull Requests review fixs * wording * Wording * Use the Spring Cloud Context version from jhipster-dependencies * Use the version from jhipster-dependencies * Wording * Corrected issues with Postgresql database not deploying correctly * Switched out NLB for ALB * Split scaling out from performance question * Added nesting supporting into repo name * Added stack name checking * Corrected issues with the path lookup * Code review feedback * Fixed linting errors * Swapped out bootRepackage for bootWar
ggotti
added a commit
that referenced
this pull request
Mar 5, 2018
* new aws-containers sub-generator New AWS sub-generator which uses Amazon ECS to run JHipster monolithic applications. Closes jhipster#6773 * external constants to localised file Moved constants from index.js into a separate aws-containers generator specific file * testing and linting errors * add back missing constants from aws-containers * address issue where the base.template is not being read correctly * Pull Requests review fixs * wording * Wording * Use the Spring Cloud Context version from jhipster-dependencies * Use the version from jhipster-dependencies * Wording * Corrected issues with Postgresql database not deploying correctly * Switched out NLB for ALB * Split scaling out from performance question * Added nesting supporting into repo name * Added stack name checking * Corrected issues with the path lookup * Code review feedback * Fixed linting errors * Swapped out bootRepackage for bootWar
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.
Split scaling out from performance and correction for Postgres provisioning