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.

Sign up for GitHub

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

Fix Python linter errors - Part 1 #310

Merged
merged 7 commits into from
Nov 8, 2017
Merged

Fix Python linter errors - Part 1 #310

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
886ec9e
Fix Python linter errors for - Part 1
digitalfishpond Oct 30, 2017
764bf6f
Merge branch 'master' into fix-lint-py-1
kornicameister Oct 31, 2017
045587c
Merge branch 'master' into fix-lint-py-1
kornicameister Oct 31, 2017
6f48c04
Merge branch 'master' into fix-lint-py-1
digitalfishpond Nov 8, 2017
fb657a4
Merge remote-tracking branch 'upstream/master' into fix-lint-py-1
digitalfishpond Nov 8, 2017
a1a54a1
Fix Python linter error - Part 1
digitalfishpond Nov 8, 2017
fda4630
Merge remote-tracking branch 'origin/fix-lint-py-1' into fix-lint-py-1
digitalfishpond Nov 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/usr/bin/env python
# coding=utf-8

# (C) Copyright 2017 Hewlett Packard Enterprise Development LP
#
Expand All @@ -14,12 +15,9 @@
# License for the specific language governing permissions and limitations
# under the License.





def main():
pass


if __name__ == '__main__':
main()
39 changes: 18 additions & 21 deletions ci.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/usr/bin/env python
# coding=utf-8

# (C) Copyright 2017 Hewlett Packard Enterprise Development LP
#
Expand Down Expand Up @@ -172,7 +173,7 @@ def upload_manifest(pipeline, voting, uploaded_files, dirty_modules, files, tags
manifest_dict = print_env(pipeline, voting, to_print=False)
manifest_dict['modules'] = {}
for module in dirty_modules:
manifest_dict['modules'][module] = {'files': []}
manifest_dict['modules'][module] = {'files': []}
for f in files:
if module in f:
if 'init' not in module and 'init' not in f or 'init' in module and 'init' in f:
Expand All @@ -182,7 +183,7 @@ def upload_manifest(pipeline, voting, uploaded_files, dirty_modules, files, tags
for f, url in uploaded_files.iteritems():
if module in f:
if 'init' not in module and 'init' not in f or 'init' in module and 'init' in f:
manifest_dict['modules'][module]['uploaded_log_file'][f] = url
manifest_dict['modules'][module]['uploaded_log_file'][f] = url

manifest_dict['run_logs'] = {}
for f, url in uploaded_files.iteritems():
Expand All @@ -197,7 +198,6 @@ def upload_manifest(pipeline, voting, uploaded_files, dirty_modules, files, tags

def upload_files(log_dir, bucket):
uploaded_files = {}
blob = bucket.blob(log_dir)
for f in os.listdir(log_dir):
file_path = log_dir + f
if os.path.isfile(file_path):
Expand Down Expand Up @@ -347,7 +347,7 @@ def kill(signal, frame):
def run_push(modules):
if os.environ.get('TRAVIS_SECURE_ENV_VARS', None) != "true":
print('No push permissions in this context, skipping!')
print('Not pushing: %r' % modules)
print('Not pushing: {0!r}'.format(modules))
Copy link
Member

Choose a reason for hiding this comment

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

You don't need 0 here.

return

username = os.environ.get('DOCKER_HUB_USERNAME', None)
Expand Down Expand Up @@ -384,7 +384,7 @@ def kill(signal, frame):
def run_readme(modules):
if os.environ.get('TRAVIS_SECURE_ENV_VARS', None) != "true":
print('No Docker Hub permissions in this context, skipping!')
print('Not updating READMEs: %r' % modules)
print('Not updating READMEs: {0!r}'.format(modules))
Copy link
Member

Choose a reason for hiding this comment

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

You don't need 0 here.

return

log_dir = BUILD_LOG_DIR
Expand Down Expand Up @@ -438,9 +438,9 @@ def update_docker_compose(modules, pipeline):
try:
with open(CI_COMPOSE_FILE, 'w') as docker_compose:
yaml.dump(compose_dict, docker_compose, default_flow_style=False)
except:
except (RuntimeError, IOError):
Copy link
Member

@matrixik matrixik Nov 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Use EnvironmentError instead of IOError
https://docs.python.org/2/library/exceptions.html#exceptions.EnvironmentError
and catch also error from yaml

raise FileWriteException(
'Error writing CI dictionary to %s' % CI_COMPOSE_FILE
'Error writing CI dictionary to {}'.format(CI_COMPOSE_FILE)
)


Expand All @@ -449,7 +449,7 @@ def load_yml(yml_path):
with open(yml_path) as compose_file:
compose_dict = yaml.safe_load(compose_file)
return compose_dict
except:
except(RuntimeError, IOError):
Copy link
Member

Choose a reason for hiding this comment

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

Use EnvironmentError instead of IOError
https://docs.python.org/2/library/exceptions.html#exceptions.EnvironmentError
and catch also error from yaml

raise FileReadException('Failed to read %s', yml_path)


Expand Down Expand Up @@ -482,11 +482,11 @@ def handle_pull_request(files, modules, tags, pipeline):
cool_test_mapper = {
'smoke': {
METRIC_PIPELINE_MARKER: run_smoke_tests_metrics,
LOG_PIPELINE_MARKER: lambda : print('No smoke tests for logs')
LOG_PIPELINE_MARKER: lambda: print('No smoke tests for logs')
},
'tempest': {
METRIC_PIPELINE_MARKER: run_tempest_tests_metrics,
LOG_PIPELINE_MARKER: lambda : print('No tempest tests for logs')
LOG_PIPELINE_MARKER: lambda: print('No tempest tests for logs')
}
}

Expand All @@ -510,19 +510,17 @@ def pick_modules_for_pipeline(modules, pipeline):
other_modules = [
'storm'
]
print('modules: %s \n pipeline_modules: %s' % (modules, pipeline_modules))
print('modules: {} \n pipeline_modules: {}'.format(modules, pipeline_modules))

# iterate over copy of all modules that are planned for the build
# if one of them does not belong to active pipeline
# remove from current run
for m in modules[::]:
if m not in pipeline_modules:
if m in other_modules:
print('%s is not part of either pipeline, but it will be build anyway' % m)
print('{} is not part of either pipeline, but it will be build anyway'.format(m))
continue
print('Module %s does not belong to %s, skipping' % (
m, pipeline
))
print('Module {} does not belong to {}, skipping'.format(m, pipeline))
modules.remove(m)

return modules
Expand Down Expand Up @@ -768,7 +766,7 @@ def kill(signal, frame):

signal.signal(signal.SIGINT, kill)
time_delta = 0
while(True):
while True:
poll = p.poll()
if poll is None:
if time_delta == 1500:
Expand All @@ -787,8 +785,7 @@ def kill(signal, frame):


def handle_other(files, modules, tags, pipeline):
print('Unsupported event type "%s", nothing to do.' % (
os.environ.get('TRAVIS_EVENT_TYPE')))
print('Unsupported event type "{}", nothing to do.'.format(os.environ.get('TRAVIS_EVENT_TYPE')))
Copy link
Member

Choose a reason for hiding this comment

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

I know max length is set to 120 but I would still prefer to stick closer to 80 chars for better diffs on github and shorter lines in editor.



def print_env(pipeline, voting, to_print=True):
Expand All @@ -798,7 +795,7 @@ def print_env(pipeline, voting, to_print=True):
'TRAVIS_PULL_REQUEST': os.environ.get('TRAVIS_PULL_REQUEST'),
'TRAVIS_PULL_REQUEST_SHA': os.environ.get('TRAVIS_PULL_REQUEST_SHA'),
'TRAVIS_PULL_REQUEST_SLUG': os.environ.get('TRAVIS_PULL_REQUEST_SLUG'),
'TRAVIS_SECURE_ENV_VARS': os.environ.get('TRAVIS_SECURE_ENV_VARS'),
'TRAVIS_SECURE_ENV_VARS': os.environ.get('TRAVIS_SECURE_ENV_VARS'),
'TRAVIS_EVENT_TYPE': os.environ.get('TRAVIS_EVENT_TYPE'),
'TRAVIS_BRANCH': os.environ.get('TRAVIS_BRANCH'),
'TRAVIS_PULL_REQUEST_BRANCH': os.environ.get('TRAVIS_PULL_REQUEST_BRANCH'),
Expand All @@ -808,7 +805,7 @@ def print_env(pipeline, voting, to_print=True):
'TRAVIS_JOB_NUMBER': os.environ.get('TRAVIS_JOB_NUMBER'),

'CI_PIPELINE': pipeline,
'CI_VOTING': voting }}
'CI_VOTING': voting}}

if to_print:
print (json.dumps(environ_vars, indent=2))
Expand Down Expand Up @@ -856,7 +853,7 @@ def main():
if voting:
raise
else:
print('%s is not voting, skipping failure' % pipeline)
print('{} is not voting, skipping failure'.format(pipeline))
finally:
output_docker_ps()
output_docker_logs()
Expand Down