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

cmd/ie/commands: Use RunE instead of Run to make code simpler #83

Open
wants to merge 246 commits into
base: main
Choose a base branch
from

Conversation

blanquicet
Copy link
Member

Using RunE instead of Run prevents us from logging and printing the error, manually displaying the help and force-exiting every time. In other words, we can pass from this:

	logging.GlobalLogger.Errorf("Error: Invalid environment variable format: %s", environmentVariable)
	fmt.Printf("Error: Invalid environment variable format: %s", environmentVariable)
	cmd.Help()
	os.Exit(1)

To this:

	return fmt.Errorf("invalid environment variable format: %s", environmentVariable)

Doing so, the error will be logged and printed here:

https://github.com/blanquicet/InnovationEngine/blob/44b942573b8cdd7fbc999739074a75cb79af7193/cmd/ie/commands/root.go#L44-L45

And the cobra library will take care of printing the help.

rgardler-msft and others added 30 commits May 18, 2022 15:23
* Created main.py and am doing a test commit

* test commit

* test commit

* Parses a markdown file and pulls out headings, code blocks, and paragraphs

* took away a few magic values & cleaned up code a bit

* added the initial executor code complete with repl behavior, by no means complete and it will have bugs but it would be good to get some eyes on it before I go too much further

* added a few more comments and deleted a few unused functions

* Added testing directions of using python3 main.py to test

* Added automated testing functionality with fuzzy matching

* cleaned up code a bit and added test functionality

* testing

* Added github actions for automated testing

* Fixing github actions

* bug fix for actions

* Added push to main as a trigger

* bug fix

* testing

* changed file type to yml

* testing

* fixed bug with fuzzywuzzy

* changing sensitivity so that actions should fail as there is a warning

* Changing Sensitivity and adjust actions test

* testing

* test should pass now

* Trying to enable manual workflow trigger

* testing github actions

* Changing default test

* added the ability to set command line variables from both comments and .ini files which are key value pairs

* updated README.md and added new test scripts

* fixed test file and changed github action to point to test file not readme

* Hardedned program to deal with broken markdown files, there is still no good way to deal with interactive bash commands though

* added some exception handling so the program doesn't crash on a timeout

* Changed Parser file class to MarkdownParser to avoid name clash with std parser, added test scripts folder for testing, and added some additional error handling for commands

* removing exit(1) from test section as it terminates early if multiple files are being tested

* removed exit(1) from main when file does not parse correctly

* removed exit 1 to stop early failures with multiple files

* adding random string to end of resource group in testing section so that there are not name conflicts. Catch is the document itself has to use MY_RESOURCE_GROUP_NAME as the variable for testing

* cleaned up file structure and fixed a few bugs in Readme.md

* added verion locks to requirements.txt, deleted hardcoded vm create document, added .vscode to .gitignore, and modified test script to work based on new location
* Improve getting started docs

1. fix capitlization error 
2. use virtual environment
3. separate installation from running

* Switch to interactive mode

Co-authored-by: jasonmesser7 <91570951+jasonmesser7@users.noreply.github.com>

---------

Co-authored-by: jasonmesser7 <91570951+jasonmesser7@users.noreply.github.com>
* first draft of AKS quick start with event grid notfications

* Added tutorial with AKS create + Subscribing to event grid notifications

* Add tests for all steps, plus a few docs improvements.

---------

Co-authored-by: Ross Gardler <nope@no.no>
…lti tenancy, however we need this signal for an action to fail and block a PR (Azure#15)
vmarcella and others added 22 commits October 13, 2023 14:06
* [update] sed statement to use single quotations to avoid expansion during variable rendering and format document.

* [update] command rendering failures to be reported.

* [fix] resource URIs not being found.
* Fixed errors on AKS doc

* Fixed errors on AKS doc

* Fixed errors on AKS doc

* Fixed scenario bugs and cleaned text

* Reverted dns name change

* Fixed styling issues

* Reverting moving around variable declarations and small text changes

---------

Co-authored-by: Mitchell Bifeld <mbifeld@Mitchell-Surface>
Fix multi-line command rendering and disable inline rendering
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.8.0 to 0.17.0.
- [Commits](golang/net@v0.8.0...v0.17.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
…rg/x/net-0.17.0

Bump golang.org/x/net from 0.8.0 to 0.17.0
* [fix] remove horizontal align

* [update] command output to not be tabbed
@vmarcella
Copy link
Member

@blanquicet Apologies for responding to this PR so late -- These changes look good to me. Could you sync your branch with main to resolve the merge conflicts so that I could merge this PR for you? Thanks in advance.

Signed-off-by: Jose Blanquicet <josebl@microsoft.com>
Signed-off-by: Jose Blanquicet <josebl@microsoft.com>
Signed-off-by: Jose Blanquicet <josebl@microsoft.com>
Signed-off-by: Jose Blanquicet <josebl@microsoft.com>
Signed-off-by: Jose Blanquicet <josebl@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

None yet

10 participants