Skip to content

updating autoReload feature#345

Open
aceppaluni wants to merge 6 commits into
hyperledger-firefly:mainfrom
aceppaluni:main
Open

updating autoReload feature#345
aceppaluni wants to merge 6 commits into
hyperledger-firefly:mainfrom
aceppaluni:main

Conversation

@aceppaluni

@aceppaluni aceppaluni commented Aug 22, 2025

Copy link
Copy Markdown

This PR updates autoReload

Signed-off-by: Angelina <aceppaluni@gmail.com>
@aceppaluni aceppaluni marked this pull request as ready for review August 25, 2025 13:37

@EnriqueL8 EnriqueL8 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @aceppaluni a few comments, the autoReload was put in the wrong place

Have you tested this by changing something the config in the runtime folder of a running stack and seeing it reload it?

Comment thread cmd/init.go Outdated
Comment thread pkg/types/firefly_config.go Outdated
…flag description

Signed-off-by: Angelina <aceppaluni@gmail.com>
@EnriqueL8

Copy link
Copy Markdown
Contributor

@aceppaluni There is an error

Error: ../internal/stacks/stack_manager.go:463:11: config.AutoReload undefined (type *"github.com/hyperledger/firefly-cli/pkg/types".FireflyConfig has no field or method AutoReload)

Signed-off-by: Angelina <aceppaluni@gmail.com>
@aceppaluni

aceppaluni commented Oct 24, 2025

Copy link
Copy Markdown
Author

Ran the unit tests locally in internal/stacks and verified that AutoReload is applied inside CoreConfig.

@aceppaluni

Copy link
Copy Markdown
Author

Hi @EnriqueL8
Are there additional changes that need to be made for this PR?

@EnriqueL8 EnriqueL8 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good - one small comment

Comment thread internal/stacks/autoreload_test.go Outdated
Signed-off-by: Angelina <aceppaluni@gmail.com>
@aceppaluni

Copy link
Copy Markdown
Author

@EnriqueL8 Comment has been removed :)

Comment thread internal/stacks/autoreload_test.go Outdated
Signed-off-by: Angelina <aceppaluni@gmail.com>
@aceppaluni

Copy link
Copy Markdown
Author

@EnriqueL8 Was there anything further needed for this PR?

@EnriqueL8

EnriqueL8 commented Dec 1, 2025

Copy link
Copy Markdown
Contributor

@aceppaluni I think we are good to merge here - once the builds are fixed

@aceppaluni

aceppaluni commented Dec 1, 2025

Copy link
Copy Markdown
Author

@EnriqueL8 Sounds good!
Please let me know if you need anything further from me to correct the builds :)

@EnriqueL8

Copy link
Copy Markdown
Contributor

@aceppaluni all the builds are failing here --- FAIL: TestMockConfigWithAutoReload (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xa922b3]

goroutine 36 [running]:

Signed-off-by: Angelina <aceppaluni@gmail.com>
@aceppaluni

Copy link
Copy Markdown
Author

Hi @EnriqueL8 I am attempting to fix the build issues but when I push a new change I do not see them right away. Is there a way to edit/change this feature so I can view the builds?

@EnriqueL8

EnriqueL8 commented Dec 2, 2025

Copy link
Copy Markdown
Contributor

@aceppaluni you can run the tests locally with act which is a CLI to run Github Workflows - since you are an external contributor I have to manually approved the workflows to run

@EnriqueL8

Copy link
Copy Markdown
Contributor

https://github.com/nektos/act

you want to run the e2e.yml one in .github/workflows

@aceppaluni

Copy link
Copy Markdown
Author

@EnriqueL8 No problem, I understand. Thank you!!

@EnriqueL8

Copy link
Copy Markdown
Contributor

@aceppaluni just checking if you need anything from me here or are you trying to fix the tests??

@aceppaluni

aceppaluni commented Dec 10, 2025

Copy link
Copy Markdown
Author

@EnriqueL8
Thanks for checking! I believe the FireFly core tests/behavior may need to be updated to match this new default but I am not certain at this time. I am attempting to see how long it takes for the file to propagate into the Docker container. I will report back :)

@aceppaluni

aceppaluni commented Dec 11, 2025

Copy link
Copy Markdown
Author

@EnriqueL8 I am not able to determine the amount of time. My device is not allowing me to complete the testing and I am continuously hitting this error

Error: dial tcp 127.0.0.1:5100: connectex: No connection could be made because the target machine actively refused it.

I attempted to run in Ubuntu as well and I am getting an additional error "ff command not found"

@EnriqueL8

Copy link
Copy Markdown
Contributor

Okay no worries - I'll pick it up from here

@aceppaluni

Copy link
Copy Markdown
Author

@EnriqueL8 Not a problem. I did do a retry in case it was an issue with my system and I get a batch error.

Error: http://127.0.0.1:5000/api/v1/network/organizations/self?confirm=true [408] {"error":"FF00166: The request with id 'f72PpmtA' timed out after 0.69ms: FF10327: Message size 1.20kb is too large for the max broadcast batch size of 0.00kb"}

Please let me know if there's anything further you need from me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants