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.

Sign up for GitHub

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

Jump to bottom

Remove localosmosis Dockerfile #2529

Merged
merged 2 commits into from
Aug 30, 2022
Merged

Remove localosmosis Dockerfile #2529

merged 2 commits into from
Aug 30, 2022

Conversation

niccoloraspa
Copy link
Member

@niccoloraspa niccoloraspa commented Aug 27, 2022 •
edited
Loading

Addresses: #1679 [1/2]

What is the purpose of the change

This PR removes the Dockerfile created for localosmosis, favoring the re-use of the main Dockerfile.

I have also restructured a bit the scripts/setup.sh script.

I will remove also the Dockerfile for the mainnet exported localosmosis but it will take a little bit more time as the logic is more complicated.

Testing and Verifying

Run localosmosis as before:

make localnet-build
make localnet-start
# or `make localnet-startd` to run localosmosis in detached mode
make localnet-stop

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? no
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no
  • How is the feature or change documented? not applicable

@niccoloraspa niccoloraspa requested a review from a team August 27, 2022 21:42
@niccoloraspa niccoloraspa added the A:backport/v11.x backport patches to v11.x branch label Aug 27, 2022
czarcas7ic
czarcas7ic reviewed Aug 29, 2022
View reviewed changes
Copy link
Member

@czarcas7ic czarcas7ic left a comment

Choose a reason for hiding this comment

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

So in my local test, there is no way to stop and start the localosmosis instance and retain the data. Is that the same for you? Imo this is a necessary capability

@ValarDragon
Copy link
Member

Agreed @czarcas7ic, thats def a necessary capability

@niccoloraspa
Copy link
Member Author

Okay! I thought It was not a requirement.
I'll revert the changes on the volumes

czarcas7ic
czarcas7ic approved these changes Aug 29, 2022
View reviewed changes
Copy link
Member

@czarcas7ic czarcas7ic left a comment

Choose a reason for hiding this comment

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

Tested locally, good to go! 😄

@ValarDragon ValarDragon merged commit 09b18a5 into main Aug 30, 2022
@ValarDragon ValarDragon deleted the feat/remove-localosmosis-dockerimage branch August 30, 2022 09:20
@mergify mergify bot mentioned this pull request Aug 30, 2022
Merged
mergify bot pushed a commit that referenced this pull request Aug 30, 2022
@niccoloraspa @mergify
Remove localosmosis Dockerfile (#2529)
0cd7971 
* Remove localosmois Dockerfile

* Retain .osmosisd folder

(cherry picked from commit 09b18a5)
ValarDragon pushed a commit that referenced this pull request Aug 30, 2022
@mergify @niccoloraspa
Remove localosmosis Dockerfile (#2529) (#2547)
c600a67 
* Remove localosmois Dockerfile

* Retain .osmosisd folder

(cherry picked from commit 09b18a5)

Co-authored-by: Niccolo Raspa <6024049+niccoloraspa@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ValarDragon ValarDragon ValarDragon left review comments

@czarcas7ic czarcas7ic czarcas7ic approved these changes

Assignees
No one assigned
Labels
A:backport/v11.x backport patches to v11.x branch T:build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@niccoloraspa @ValarDragon @czarcas7ic