这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@0roman
Copy link
Contributor

@0roman 0roman commented Mar 13, 2018

No description provided.

@0roman 0roman requested a review from dorpvom March 13, 2018 14:54
Copy link
Member

@dorpvom dorpvom left a comment

Choose a reason for hiding this comment

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

Script is not well thought out. Some parts are not needed, others are missing. The tests fail and not all use cases are working.

signal.signal(signal.SIGTERM, shutdown)


def main():
Copy link
Member

Choose a reason for hiding this comment

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

The command_line_options have to be given as parameter (and propagated to program_setup).

analysis_service = AnalysisScheduler(config=config)
analysis_service.shutdown()
logging.info('Restart Analysis component')
analysis_service = AnalysisScheduler(config=config)
Copy link
Member

Choose a reason for hiding this comment

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

Why Start the Scheduler twice?
Also: The CompareScheduler has to also be started, as it also does view sync.

@@ -0,0 +1,43 @@
import logging
Copy link
Member

Choose a reason for hiding this comment

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

Script missing shebang and execution permissions

Copy link
Member

Choose a reason for hiding this comment

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

Still missing ...

try:
pass
except KeyboardInterrupt:
break
Copy link
Member

Choose a reason for hiding this comment

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

I don't want to kill this manually. Instead just wait some seconds and then start the teardown



@pytest.mark.parametrize('script', [init_database, migrate_database, update_statistic, update_variety_data])
@pytest.mark.parametrize('script', [init_database, migrate_database, update_statistic, update_variety_data, restart_analysis_and_reload_templates])
Copy link
Member

Choose a reason for hiding this comment

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

test fails. See above for reasons.

PROGRAM_DESCRIPTION = 'Restart FACT Analysis Component and reload Analysis Templates'


def shutdown(signum, frame):
Copy link
Member

Choose a reason for hiding this comment

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

not needed if script self destroys (also applies for lines 20 + 21)

Copy link
Member

@dorpvom dorpvom left a comment

Choose a reason for hiding this comment

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

One request still open. Please change and issue a new jenkins build.

@@ -0,0 +1,43 @@
import logging
Copy link
Member

Choose a reason for hiding this comment

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

Still missing ...

@codecov-io
Copy link

codecov-io commented Mar 23, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@9b32efb). Click here to learn what that means.
The diff coverage is 96.15%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #82   +/-   ##
=========================================
  Coverage          ?   94.12%           
=========================================
  Files             ?      291           
  Lines             ?    14102           
  Branches          ?        0           
=========================================
  Hits              ?    13273           
  Misses            ?      829           
  Partials          ?        0
Impacted Files Coverage Δ
...rc/test/acceptance/run_scripts/test_run_scripts.py 79.31% <100%> (ø)
src/restart_analysis_and_reload_templates.py 95.83% <95.83%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b32efb...9c9cbe6. Read the comment docs.

@dorpvom
Copy link
Member

dorpvom commented Apr 17, 2018

Before merging into master, master has to be merged into this branch and the script has to be moved to a suitable location ( != src )

@dorpvom dorpvom closed this May 23, 2018
@dorpvom dorpvom deleted the restart_analysis_and_reload_templates branch May 23, 2018 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants