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

read and store return-progress output on a dedicated output file #670

Merged
merged 9 commits into from
Apr 23, 2024

Conversation

burnout87
Copy link
Collaborator

No description provided.

@burnout87
Copy link
Collaborator Author

in relation to what discussed in oda-hub/mmoda-frontend-module#250 (comment)

Copy link

codecov bot commented Apr 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.34%. Comparing base (7331d28) to head (53dd71a).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #670   +/-   ##
=======================================
  Coverage   61.34%   61.34%           
=======================================
  Files          49       49           
  Lines        8731     8732    +1     
=======================================
+ Hits         5356     5357    +1     
  Misses       3375     3375           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@burnout87
Copy link
Collaborator Author

With this, the idea is to write the outputs of the return-progress call, in dedicated files, namely:

  • query_output.json.return-progress-query-output to store the output (eg html notebook output)
  • query_output.json.return-progress-job-monitor to store the resulting job_monitor object

In addition to that, the "main" job object won't be overwritten (that is the cause of the reset of the progress bar), and consequently the content of the job_monitor.json file. The job output of the return-progress call will be stored in the mentioned above.

@burnout87 burnout87 marked this pull request as ready for review April 15, 2024 20:09
@burnout87
Copy link
Collaborator Author

tests on the nb2workflow plugin fail, but are addressed with oda-hub/dispatcher-plugin-nb2workflow#100

@volodymyrss
Copy link
Member

Can it be instead that return_progress just adds some information to query output, not creating an entirely new output structure?

Copy link
Member

@volodymyrss volodymyrss left a comment

Choose a reason for hiding this comment

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

Can it be instead that return_progress just adds some information to query output, not creating an entirely new output structure?

@burnout87
Copy link
Collaborator Author

With this, the idea is to write the outputs of the return-progress call, in dedicated files, namely:

* `query_output.json.return-progress-query-output` to store the output (eg html  notebook output)

* `query_output.json.return-progress-job-monitor` to store the resulting job_monitor object

In addition to that, the "main" job object won't be overwritten (that is the cause of the reset of the progress bar), and consequently the content of the job_monitor.json file. The job output of the return-progress call will be stored in the mentioned above.

EDIT

The query output of 'return-progress' is not in a separate file, and the previous behavior has been re-established.

The only difference is that the job-monitor will not be overwritten, thus preventing the progress bar from being reset.

@burnout87 burnout87 requested a review from volodymyrss April 22, 2024 16:42
@burnout87
Copy link
Collaborator Author

the nb2workflow plugin is now failing in test for the ontology, any ideas why ?

@dsavchenko
Copy link
Member

dsavchenko commented Apr 23, 2024

the nb2workflow plugin is now failing in test for the ontology, any ideas why ?

Here is a test adaptation oda-hub/dispatcher-plugin-nb2workflow#101

@burnout87 burnout87 merged commit 8d519bf into master Apr 23, 2024
17 checks passed
@dsavchenko dsavchenko deleted the return-progress-dedicated-output-file branch April 23, 2024 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants