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

region-info Fact.find() should throw exception if fact not found #3194

Closed
1 of 5 tasks
chelma-amzn opened this issue Jul 3, 2019 · 2 comments Β· Fixed by #5166
Closed
1 of 5 tasks

region-info Fact.find() should throw exception if fact not found #3194

chelma-amzn opened this issue Jul 3, 2019 · 2 comments Β· Fixed by #5166
Assignees
Labels
@aws-cdk/region-info Related to AWS Region information feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on.

Comments

@chelma-amzn
Copy link

  • I'm submitting a ...

    • πŸͺ² bug report
    • πŸš€ feature request
    • πŸ“š construct library gap
    • ☎️ security issue or vulnerability => Please see policy
    • ❓ support request => Please see note at the top of this template.
  • What is the current behavior?
    If the current behavior is a πŸͺ²bugπŸͺ²: Please provide the steps to reproduce

The current behavior is that when a user uses the @aws-cdk/region-info's Fact.find() method, it may return an undefined result. This forces users to perform further checking if they want to pass the result to their own code.

`
const fact = Fact.find(...); // type string?

if (!fact) throw new Error('fact must be provided');

// here fact has type string
`

  • What is the expected behavior (or behavior of feature suggested)?

The Fact class should define a method that throws an exception if the fact is not found.

  • What is the motivation / use case for changing the behavior or adding this feature?

This enables users to avoid boilerplate code.

  • Please tell us about your environment:

N/A

  • Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. associated pull-request, stackoverflow, gitter, etc)
@chelma-amzn chelma-amzn added the needs-triage This issue or PR still needs to be triaged. label Jul 3, 2019
@RomainMuller RomainMuller self-assigned this Aug 26, 2019
@SomayaB SomayaB added feature-request A feature should be added or improved. @aws-cdk/region-info Related to AWS Region information good first issue Related to contributions. See CONTRIBUTING.md labels Oct 11, 2019
@SomayaB
Copy link
Contributor

SomayaB commented Oct 11, 2019

Hi @chelma-amzn, thank you for submitting this request! This seems totally reasonable, and not too difficult to implement. If you would like to submit a PR one of us will review it, otherwise someone will update this issue when there is further information!

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Oct 11, 2019
stephendwu added a commit to stephendwu/aws-cdk that referenced this issue Nov 22, 2019
* Throw exception when no fact is found given a region, and fact name.

closes aws#3194
stephendwu added a commit to stephendwu/aws-cdk that referenced this issue Nov 22, 2019
* Throw exception when no fact is found given a region, and fact name.

closes aws#3194
stephendwu added a commit to stephendwu/aws-cdk that referenced this issue Nov 22, 2019
* Throw exception when no fact is found given a region, and fact name.

closes aws#3194
stephendwu added a commit to stephendwu/aws-cdk that referenced this issue Nov 22, 2019
* Throw exception when no fact is found given a region, and fact name.

closes aws#3194
stephendwu added a commit to stephendwu/aws-cdk that referenced this issue Nov 22, 2019
* Throw exception when no fact is found given a region, and fact name.

closes aws#3194
@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Nov 22, 2019
@stephendwu
Copy link
Contributor

@chelma-amzn Posted a question in the glitter group, but are we trying to define a second method that will throw, or have this original method throw?

stephendwu added a commit to stephendwu/aws-cdk that referenced this issue Nov 23, 2019
 * New method that throws an exception when the Fact value, given a name and region, are not found.

closes aws#3194
stephendwu added a commit to stephendwu/aws-cdk that referenced this issue Nov 27, 2019
 * New method that throws an exception when the Fact value, given a name and region, are not found.

closes aws#3194
stephendwu added a commit to stephendwu/aws-cdk that referenced this issue Nov 27, 2019
 * New method that throws an exception when the Fact value, given a name and region, are not found.

closes aws#3194
@mergify mergify bot closed this as completed in #5166 Dec 10, 2019
mergify bot pushed a commit that referenced this issue Dec 10, 2019
* New method that throws an exception when the Fact value, given a name and region, are not found.

closes #3194
ed-at-work pushed a commit to ed-at-work/aws-cdk that referenced this issue Dec 17, 2019
* New method that throws an exception when the Fact value, given a name and region, are not found.

closes aws#3194
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/region-info Related to AWS Region information feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on.
Projects
None yet
4 participants