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

Triage MySQL 8 geospatial query compatibility with freezing-sync #56

Closed
obscurerichard opened this issue Dec 1, 2024 · 4 comments · Fixed by freezingsaddles/freezing-web#314 or freezingsaddles/freezing-model#41
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@obscurerichard
Copy link
Member

obscurerichard commented Dec 1, 2024

In Exception in all/tracks.geojson - suspected MySQL 8 incompatibility with SQLAlchemy #295 @obscurerichard found a troubling exception that might require a whole chain of library upgrades to resolve. In freezingsaddles/freezing-web#281 we upgraded to MySQL 8, but maybe without adequate testing.

We should triage freezing-sync to ensure that it does not run into trouble withpymysql.err.OperationalError: (1305, 'FUNCTION freezing.AsBinary does not exist') or similar error messages. The same references and leads that apply to freezingsaddles/freezing-web#295 will likely apply here. The TLDR is that if we do have a problem it might require us to upgrade or patch SQLAlchemy and friends, which is likely to be a huge pain.

Strategies

Integration testing

We could establish some integration tests that inject data into freezing-sync, possibly via beanstalkd messages, maybe with real live data from Strava, that can help us verify that ingesting Strava activities, including their tracks, is still working properly.

This is high effort, but low risk.

Live testing

We could alter the start and end dates of the competition to temporarily have activities recorded from Strava get recorded, then clear them out of the database by doing a partial reset of the rides.

This is low effort, but medium risk and might be confusing to users.

@merlinorg
Copy link
Contributor

So you can sync_activities locally without a lot of effort, which should exercise all the sync-related geo code. My notes for doing this are on an archived laptop but I think it's just a case of grabbing a key and rider id.

@merlinorg
Copy link
Contributor

@obscurerichard my notes for testing sync locally.

export DEBUG=true
export SQLALCHEMY_URL=...
export STRAVA_CLIENT_ID=...
export STRAVA_CLIENT_SECRET=...
export VISUAL_CROSSING_API_KEY=...
export START_DATE=2024-11-01T00:00:00-05:00
export END_DATE=2025-03-19T23:59:59-04:00
cd freezing/sync
python3.9 -m cli.sync_activities --athlete-id 12345
python3.9 -m cli.sync_streams --athlete-id 12345

I use my own athlete id.

I set the start date to 11/1 so there would be data to pull.

I wind up creating symlinks everywhere to get the latest freezing-model from my local branch.

@obscurerichard
Copy link
Member Author

obscurerichard commented Dec 4, 2024

Thanks for the troubleshooting notes. It's ok to push a new version tag in the freezing-model repo for testing, even from an experimental branch, I do it all the time, then you don't need crazy symlinks. Even if we don't end up using that tag in the long run or it is a dead end for code that doesn't make it into the default branch, it's ok. I try to follow semver generally though.

If you install freezing-sync in a local virtualenv you can do pip install -e . in freezing-sync and then the cli options get on the path as freezing-sync-activitities and company so the gymnastics with -m freezing.sync.cli.sync_activities become superfluous.

I did a bunch of testing on this tonight and was able to confirm that the remove-geo patches do seem to work for sync at least. And the coverage of these is very small in freezing-web. So using these is a viable option, if we hack some sort of alembic migration together. Let's hold on that for a few days though because I think it will be possible given the very limited set of changes here to do a set of library upgrades. Seeing exactly where and what is involved with this has given me some new insights on this and I think it won't be too hard to fix up the chain of libraries needed to support MySQL 8.

In related testing findings, I also found that I had accidentally dropped the ride_errors table in production when doing clean-up this year 😱 . I restored the table from a schema-only backup so it should be good to go, I want to do more import testing with the mostly blank 2025 database tomorrow. I only found this when I was doing local testing that errored out!

@obscurerichard
Copy link
Member Author

The testing results I had showed that definitely:

  1. without the remove-geo patches we definitely do have a problem
  2. with these, it works
  3. The database migrations situation would need to be taken care of
  4. I'd still rather do some library upgrades and keep the real geospatial fields in the database
  5. We need way better documentation on testing freezing-sync but Get freezing-sync README.md up to the level of freezing-web README.md #24 has been open for a long time.

@obscurerichard obscurerichard moved this from To do to In progress in Freezing Saddles Dec 7, 2024
obscurerichard added a commit to freezingsaddles/freezing-compose that referenced this issue Dec 8, 2024
The other projects all got updated to help with the fallout
of freezingsaddles/freezing-web#295
and freezingsaddles/freezing-sync#56

This makes it possible to test tags for freezing-nq as well.
@github-project-automation github-project-automation bot moved this from In progress to Done in Freezing Saddles Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
Status: Done
2 participants