support redirecting for older versions of phoebe - #7
Conversation
aprsa
left a comment
aprsa
left a comment
There was a problem hiding this comment.
let's merge the two PRs and revisit? Overall this looks good to me but I would love to avoid globals...
| CORS(app) | ||
| app._verbose = True | ||
|
|
||
| class VersionRedirect(Exception): |
There was a problem hiding this comment.
no, I guess we could test with a different case locally, or test once this is in place and replace with redirect returns if necessary (that just makes the code a little muddier and more repetitive)
| phoebe.interactive_off() | ||
|
|
||
| def _get_phoebe_version_latest(): | ||
| global _phoebe_version_latest_cache |
There was a problem hiding this comment.
do we really save on performance by caching this? If so, then perhaps we should offload caching to apache, much like what we do for passband files now.
There was a problem hiding this comment.
well I think we definitely don't want to reach out to pypi on every request, so some sort of internal caching is necessary
|
|
||
| def _pbs_flush(force=False): | ||
| global _pbs_last_flush | ||
| global _pbs_last_flush, _phoebe_version_latest_cache |
There was a problem hiding this comment.
worth noting here that this will flush the internal list but the original list will still be cached by apache for up to 1 hour. See ground_truth branch README for details.
| def _pbs_flush(force=False): | ||
| global _pbs_last_flush | ||
| global _pbs_last_flush, _phoebe_version_latest_cache | ||
| if _pbs_last_flush is None or force or (datetime.now()-_pbs_last_flush).total_seconds() > (60*60): |
There was a problem hiding this comment.
isn't 1 hour a bit excessive to check for the new passband version? Perhaps 1 day or even 1 week or even 1 month, with the ability to force it?
There was a problem hiding this comment.
perhaps, but I don't want to add to another thing we have to remember to do at release. We might be able to make a call to the force within the release action itself, in which case we could just have it fetched at startup and when manually refreshed 🤔
| """ | ||
|
|
||
| if version.parse(phoebe_version) < version.parse('2.5'): | ||
| return 'tables-20-24.phoebe-project.org' |
There was a problem hiding this comment.
I'd do tables24.phoebe-project.org here
|
|
||
| return phoebe_version_request | ||
|
|
||
| def _generate_request_passband(pbr, content_request, export_inorm_tables=False, gzipped=False, save=True): |
There was a problem hiding this comment.
this requires merging with the other PR
2 participants
This implements in-flask redirecting to the appropriate subdomain/container running the same server code but a different version of phoebe. It does so by:
?phoebe_versionwill run through_unpack_version_requestand compares the version > subdomain mapping (intables_subdomain) between the requested-version of PHOEBE with the version running on this instance of the server. If they are different, flask will redirect to the subdomain mapped from the requested-version of PHOEBE (which will then run the same logic and see that it is a match, and continue to process the request).tables.phoebe-project.orgremains the publicly accessible endpoint used by the website and PHOEBE code (and will going forward). As new versions of PHOEBE motivate new subdomains/containers, we should update the logic intables_subdomainto create a new subdomain for what was previously covered by the default and deploy to all running instances.