Comments (8)
Can you attach a log reproducing? The line numbers in the stack trace don't match the current repo. The error is indeed occuring because self.git_remote
is set to "?", but the state shouldn't be stale. The remote and branch are set in _find_current_branch()
. The remote itself is set by querying the configuration based on that branch, or its parsed from the response to git branch
if the head is detached. If you switch to a branch that has no upstream remote set in git config then an exception will correctly be raised.
FWIW, the update manager isn't really intended to be used in this manner. Its primary purpose is to provide updates from a remote on a static branch. Manually modifying the repo could certainly confuse it.
from moonraker.
Wow, thanks for the super-quick response!
Can you attach a log reproducing? The line numbers in the stack trace don't match the current repo.
Sorry, I've been working with a codebase riddled with extra debug log statements. Here's a full log from unchanged code:
moonraker.log
Turns out, simply restarting is not sufficient to reproduce from clean state. I can get it to throw the exceptions when I force a refresh, but that alone doesn't save the broken state. Race condition maybe?
The error is indeed occuring because
self.git_remote
is set to "?", but the state shouldn't be stale. The remote and branch are set in_find_current_branch()
. The remote itself is set by querying the configuration based on that branch, or its parsed from the response togit branch
if the head is detached. If you switch to a branch that has no upstream remote set in git config then an exception will correctly be raised.
Sure, I get that, but it seems to have worked more cleanly in the past, i.e:
- this condition (
self.git_remote == '?'
) would only result in error message - the other checks would be performed
- full validation response would be returned to a calling client
Now, no information about the actual problem shows up in Mainsail or fluidd - only the recovery button is highlighted and the info message remains stale:
FWIW, the update manager isn't really intended to be used in this manner. Its primary purpose is to provide updates from a remote on a static branch. Manually modifying the repo could certainly confuse it.
It didn't occur to me that I'm doing anything unusual. I just had it running while working on some Klipper code in a branch. I know you only have so much time to work on this, so I'd be happy to fix this regression, unless you say "nah, I'm familiar with the codebase, I already know how to fix it".
It's also worrying that the broken status was somehow saved and prevented a refresh even after switching back to master
. Even if you decide that update_manager must not be used with development repos, then there's a potential problem somewhere in there.
Cheers
KD
from moonraker.
BTW I still have one more corrupted repo state in storage, so I backed up the database in case it would help with debugging.
from moonraker.
I don't think this is a bug or a regression. Switching to a new branch that has an untracked remote would have still raised an exception prior to a7b9e57. Simply put, the update manager can't fetch the updates from a remote when there is no remote set in the git config. The way to correct the issue is to switch to a branch that is tracked, or push the branch to a remote.
from moonraker.
FWIW, i just verified. I was able to switch to a new branch on a configured repo and and reproduce the error. After switching back to master
a refresh correctly restores state.
The behavior from Moonraker is slightly different from a7b9e57, but both would have raised exceptions. In the current version the exception is raised when Moonraker attempts to verify the repo. The key branch.<branch_name>.remote
does not exist in git config. As a result, Moonraker sets the remote to "?", as its unknown. When Moonraker validates the remote before performing a fetch it raises an exception.
In the linked commit the exception would have occurred here. In this version the call to git config
would have returned status code 1, which would have raised a shell command error.
One other thing I forgot to mention, the update manager's status
endpoint has a basic "spam" filter that simply returns the current state if a new request is received within 60 seconds of a fulfilled request. Generally speaking refreshing the state isn't something that needs to occur often, and it tends to be CPU intensive (particularly on low resource machines). The filter exists to prevent several frontends/apps from connecting and requesting a status refresh all at once.
That said, Moonraker also registers the POST /machine/update/refresh
endpoint that has no such filter, however it can be used to request a refresh for a particular item, ie POST /machine/update/refresh?name=klipper
. I don't believe the primary front ends ever implemented it though.
from moonraker.
Hey, when I wrote that it was introduced in a7b9e57, I meant that the correct behavior is before it, meaning 35396a5.
But it appears the behavior on startup in 35396a5 seems to be the same. What stands out is that in 35396a5 the function _update_repo_state()
runs report_invalids( )
which sets _is_valid to False and exits gracefully. Meanwhile a7b9e57 _update_repo_state()
doesn't do that.
Still, the corruption of the saved state is freaky, but I cannot reproduce it at this time. Has to be a race condition of some sort. 🤔
from moonraker.
Hey, when I wrote that it was introduced in a7b9e57, I meant that the correct behavior is before it, meaning 35396a5.
Right, my mistake. As you stated, the behavior is the same.
What stands out is that in 35396a5 the function _update_repo_state() runs report_invalids( ) which sets _is_valid to False and exits gracefully. Meanwhile a7b9e57 _update_repo_state() doesn't do that.
That is true, however an exception in GitRepo.initialize()
would still propagate up to refresh()
.
Still, the corruption of the saved state is freaky, but I cannot reproduce it at this time. Has to be a race condition of some sort.
Are you sure the saved state was corrupted? Mainsail will at times report corrupted when the repo is invalid.
All that said, I did take a deeper look at this, and while it appears to me that its behaving as expected, the condition could be more clear to the user. In addition, the state should be saved on failure so its not reported as valid
on a restart. Commit 119f579 addresses both of those issues.
from moonraker.
Are you sure the saved state was corrupted? Mainsail will at times report corrupted when the repo is invalid.
"Corrupted" was the wrong word of me to use. The stored correctly reflected the (previous) invalid state of the repo.
But it could never be updated to the current (manually fixed with git switch master
) state, because in refresh_repo_stage
the call to _verify_repo()
happens BEFORE the call to _find_current_branch()
.
Anyways, I just pulled your new commit and this is a MASSIVE improvement in the user experience. Thanks a lot!
from moonraker.
Related Issues (20)
- Apprise update. HOT 2
- Add option to power off a power device when operating system shutdown. HOT 2
- Non working links for ids.
- Power device: add option to run custom gcode on on/off action
- [database.py:insert_item()] - Error inserting key 'instance_id' in namespace 'moonraker' HOT 5
- Login fails after upgrade HOT 9
- Missing Documentation for altering the instance_uuid HOT 3
- Add filament I’d to print history
- Mainsail cannot connect to Moonraker (connection Failed) HOT 1
- power type `uhubctl` without specifying port HOT 1
- PAM authentication support HOT 1
- Queue should be paused when non-queued file is finished printing HOT 5
- After upgrade moonraker is broken - Unable to load component (template). Caused by jinja HOT 1
- [Spoolman] Support fetching filament information from spoolman
- Print history has inaccurate “print duration” and “used filament” when using T1 HOT 2
- Update Manager Enhancement
- Repo Corrupt? HOT 1
- Allow setting MQTT ClientId in moonraker.conf HOT 1
- APC UPS - Status monitoring HOT 2
- AttributeError: module 'moonraker.components.job_queue' has no attribute 'load_component' HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from moonraker.