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

Versioning_Engine: Adding lock around the upgrader loop in the ToNewVersion method. #2941

Merged

Conversation

IsakNaslundBh
Copy link
Contributor

Issues addressed by this PR

Closes #2940

Adding a simple lock around the for loop calling the BHoMUpgraders in attempt to make it thread safe.
Simplest way of dealing with this that I could think of, but open for this to be handled differently if a better way is suggested.

Test files

Hard to provide. To test, one can ensure to have analytics logs with methods requiring versioning (or failing to deserialise for other reasons) while opening a script requiring versioning.

Changelog

Additional comments

@IsakNaslundBh IsakNaslundBh added the type:bug Error or unexpected behaviour label Nov 3, 2022
@IsakNaslundBh IsakNaslundBh self-assigned this Nov 3, 2022
Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

Code changes are sensible and resolves the issue described.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 3, 2022

@FraserGreenroyd to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance

There are 12 requests in the queue ahead of you.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 3, 2022

@FraserGreenroyd to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

There are 40 requests in the queue ahead of you.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 3, 2022

The check installer has already been run previously and recorded as a successful check. This check has not been run again at this time.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check unit-tests
@BHoMBot check ready-to-merge

1 similar comment
@FraserGreenroyd
Copy link
Contributor

@BHoMBot check unit-tests
@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 3, 2022

@FraserGreenroyd to confirm, the following actions are now queued:

  • check unit-tests
  • check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 3, 2022

@FraserGreenroyd to confirm, the following actions are now queued:

  • check unit-tests
  • check ready-to-merge

There are 1 requests in the queue ahead of you.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 3, 2022

The check unit-tests has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 3, 2022

The check ready-to-merge has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 3, 2022

FAO: @FraserGreenroyd
@FraserGreenroyd is seeking dispensation on this Pull Request to skip a required check. Please can you provide authorisation for the check to be skipped, or provide assistance as appropriate.

The check they wish to have dispensation on is code-compliance.

If you are providing dispensation on this occasion, please reply with:

@BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. 9276168902

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 3, 2022

FAO: @FraserGreenroyd
@FraserGreenroyd is seeking dispensation on this Pull Request to skip a required check. Please can you provide authorisation for the check to be skipped, or provide assistance as appropriate.

The check they wish to have dispensation on is documentation-compliance.

If you are providing dispensation on this occasion, please reply with:

@BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. 9276173070

@FraserGreenroyd
Copy link
Contributor

@BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. 9276168902

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 3, 2022

@FraserGreenroyd I have now provided a passing check on reference 9276168902 as requested.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. 9276173070

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 3, 2022

@FraserGreenroyd I have now provided a passing check on reference 9276173070 as requested.

@FraserGreenroyd FraserGreenroyd merged commit efb8b5b into main Nov 3, 2022
@FraserGreenroyd FraserGreenroyd deleted the Versioning_Engine-#2940-AddLockToToNewVersionMethod branch November 3, 2022 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Error or unexpected behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Versioning_Engine: ToNewVersion is not thread safe
2 participants