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-ore-wrapper: fix publication regressions #1047

Merged
merged 1 commit into from Jan 14, 2020

Conversation

@darkmuggle
Copy link
Contributor

darkmuggle commented Jan 14, 2020

This addresses un-intended consequences with the refactor:

  • reraise exceptions with the @retry wrapper
  • use arg.region since '--region' and '--regions' are synonyms
  • use the old meta.json 'amis' instead of 'aws'
  • introduce wrapper for ore call `run_ore' to de-duplicate the
    the logging code and support use of --config-file
@darkmuggle

This comment has been minimized.

Copy link
Contributor Author

darkmuggle commented Jan 14, 2020

/cc @jlebon this is fall out from my PR. I'll get this fixed shortly. This addresses #1045

@darkmuggle darkmuggle force-pushed the darkmuggle:pr/aws-fix branch 3 times, most recently from 3ffb0c7 to b67cc97 Jan 14, 2020
'ore', 'aliyun', 'list-regions'
if not args.region:
args.region = subprocess.check_output([
'ore', f'--config-file={args.config}' if args.config else '',

This comment has been minimized.

Copy link
@arithx

arithx Jan 14, 2020

Contributor

Might be nice to create a helper function for the config-file string since it's used in multiple places.

This comment has been minimized.

Copy link
@darkmuggle

darkmuggle Jan 14, 2020

Author Contributor

Agreed. That's next.

src/cosalib/aws.py Outdated Show resolved Hide resolved
Copy link
Member

cgwalters left a comment

Seems OK to me.

- reraise exceptions with the @retry wrapper
- use arg.region since '--region' and '--regions' are synonyms
- use the old meta.json 'amis' instead of 'aws'
@cgwalters

This comment has been minimized.

Copy link
Member

cgwalters commented Jan 14, 2020

Should this still be draft? Pipeline is still down, feels like we should try to move forward even if it's not perfect.

@darkmuggle darkmuggle force-pushed the darkmuggle:pr/aws-fix branch from b67cc97 to c678821 Jan 14, 2020
@darkmuggle darkmuggle marked this pull request as ready for review Jan 14, 2020
@darkmuggle

This comment has been minimized.

Copy link
Contributor Author

darkmuggle commented Jan 14, 2020

This is ready for review. I've put it through the paces and checked the data bay hand.

@ashcrow

This comment has been minimized.

Copy link
Member

ashcrow commented Jan 14, 2020

Should this still be draft? Pipeline is still down, feels like we should try to move forward even if it's not perfect.

Agreed 💯

@darkmuggle

This comment has been minimized.

Copy link
Contributor Author

darkmuggle commented Jan 14, 2020

Merging now.

@darkmuggle darkmuggle merged commit 3ae5871 into coreos:master Jan 14, 2020
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/jenkins/pr-merge This commit is being built
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@darkmuggle darkmuggle deleted the darkmuggle:pr/aws-fix branch Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.