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

fix: Use correct Github repository archive link #1645

Merged
merged 1 commit into from
Aug 25, 2024

Conversation

sigv
Copy link
Contributor

@sigv sigv commented Aug 22, 2024

Description

Closes: #1644

There was a mass-replacement for Lava v2 in commit 5864b7a. This resulted in the auto-download link being broken for Lavavisor. With this commit, the archive downloads can be successful again.


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • read the contribution guide
  • included the correct type prefix in the PR title, you can find examples of the prefixes below:
  • confirmed ! in the type prefix if API or client breaking change
  • targeted the main branch
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
    no functional changes impacting testing
  • updated the relevant documentation or specification, including comments for documenting Go code
    no functional changes warranting documentation changes
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

@sigv
Copy link
Contributor Author

sigv commented Aug 23, 2024

I do not see any way that changing the download URL could unexpectedly cause conflict handler to fire twice. The test case failure is reproducible locally with go test -race ./protocol/integration, so there appears to be a flaky test:

panic: sync: negative WaitGroup counter

goroutine 35237 [running]:
sync.(*WaitGroup).Add(0xc022fe22e8?, 0x14?)
	/opt/hostedtoolcache/go/1.20.14/x64/src/sync/waitgroup.go:62 +0xe5
sync.(*WaitGroup).Done(...)
	/opt/hostedtoolcache/go/1.20.14/x64/src/sync/waitgroup.go:87
github.com/lavanet/lava/v2/protocol/integration.TestSameProviderConflictReport.func5.1({0xc028e30790?, 0xc02d1ed450?}, 0xc0285068a0, 0x7aa7fe?, {0xc0280f8d80?, 0x0?})
	/home/runner/work/lava/lava/protocol/integration/protocol_test.go:1117 +0x2e5
github.com/lavanet/lava/v2/protocol/integration.(*mockConsumerStateTracker).TxConflictDetection(0xc022ac2fd0?, {0x380fd68?, 0xc0de325da0?}, 0x380fd30?, 0xc0271a82a0?, {0x37ee338?, 0xc026008240?})
	/home/runner/work/lava/lava/protocol/integration/mocks.go:55 +0x48
created by github.com/lavanet/lava/v2/protocol/rpcconsumer.(*RPCConsumerServer).relayInner
	/home/runner/work/lava/lava/protocol/rpcconsumer/rpcconsumer_server.go:1071 +0xc8e
FAIL	github.com/lavanet/lava/v2/protocol/integration	17.444s

@sigv sigv force-pushed the github-archive-v2 branch from b3e5916 to cd8fe56 Compare August 25, 2024 10:31
There was a mass-replacement for Lava v2 in commit 5864b7a.
This resulted in the auto-download link being broken for Lavavisor.
With this commit, the archive downloads can be successful again.
@sigv sigv force-pushed the github-archive-v2 branch from cd8fe56 to a11e251 Compare August 25, 2024 11:02
@omerlavanet
Copy link
Collaborator

great catch @sigv thanks

@omerlavanet omerlavanet merged commit 649e857 into lavanet:main Aug 25, 2024
24 checks passed
@sigv sigv deleted the github-archive-v2 branch August 25, 2024 14:10
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.

[Bug]: Lavavisor auto-upgrade flow cannot download archive
2 participants