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

[Build] Failing builds due to network timeouts on model loading #422

Closed
K-Kumar-01 opened this issue Jul 2, 2021 · 6 comments · Fixed by #423 or #447
Closed

[Build] Failing builds due to network timeouts on model loading #422

K-Kumar-01 opened this issue Jul 2, 2021 · 6 comments · Fixed by #423 or #447
Assignees
Labels
Difficulty: Medium Type: Enhancement ✨ Improvement to process or efficiency

Comments

@K-Kumar-01
Copy link
Collaborator

Bug Report 🐛

Currently the build fails many times when a PR is made. A re-run is required many a times for this to pass successfully. The failure is due to this line.
It involves fetching model but since azure(where the models are hosted) is inconsistent, we face the error.

Expected Behavior

The build shoud pass.

Current Behavior

The build fails,

Possible Solution

Passing an offline:true flag to options will ensure the pass.

Steps to Reproduce

  1. Make a PR.
  2. The build might fail with the error that it was not able to download the model.

Context (Environment)

Desktop

  • OS: Windows, Ubuntu
  • Browser: Chrome

Detailed Description

Passing offline:true will make the build pass.

K-Kumar-01 added a commit that referenced this issue Jul 2, 2021
Pass offline:true in loadModelManager

Signed-off-by: K-Kumar-01 <kushalkumargupta4@gmail.com>
dselman pushed a commit that referenced this issue Jul 2, 2021
Pass offline:true in loadModelManager

Signed-off-by: K-Kumar-01 <kushalkumargupta4@gmail.com>
@algomaster99
Copy link
Member

@dselman @K-Kumar-01 The change is required for more test cases than expected. You can see the latest build on master is failing.

@algomaster99 algomaster99 reopened this Jul 5, 2021
@K-Kumar-01
Copy link
Collaborator Author

@algomaster99
I think we would need to pass the offline: true option in all the instances where loadModelManger is used.

@algomaster99
Copy link
Member

Right

@K-Kumar-01
Copy link
Collaborator Author

@dselman
const modelManager = await ModelLoader.loadModelManager([modelFilePath]);
Here, the third argument is the options one.
Could you tell about second argument.

I want to understand how to pass the third argument without passing the second argument/ passing the appropriate second argument.
Thanks in advance:)

@dselman
Copy link
Contributor

dselman commented Jul 7, 2021

The options are the second argument, so you should use {offline: true} as the second argument.

Method definition:
https://github.com/accordproject/concerto/blob/master/packages/concerto-core/lib/modelloader.js#L64

@jeromesimeon jeromesimeon changed the title Failing build [Build] Failing builds due to network timeouts on model loading Jul 15, 2021
@jeromesimeon
Copy link
Member

There is an issue!! @K-Kumar-01

I still see sometimes failing tests in markdown-cli and markdown-transform but the fix in #445 should already help. I'll report when I'm done with the other packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty: Medium Type: Enhancement ✨ Improvement to process or efficiency
Projects
None yet
4 participants