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

Update Models for Filter Fields #1194

Merged
merged 22 commits into from
Mar 13, 2023

Conversation

BradleySappington
Copy link
Collaborator

closes #1185

@BradleySappington BradleySappington self-assigned this Feb 15, 2023
@BradleySappington
Copy link
Collaborator Author

closes #1186


@admin.register(Anomalies)
class AnomaliesAdmin(admin.ModelAdmin):
list_display = ('root_file_info', 'cosmic_ray_shower', 'diffraction_spike', 'excessive_saturation', 'guidestar_failure', 'persistence', 'crosstalk', 'data_transfer_error',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was going to suggest getting these from constants.py, although I guess we may not always have all anomalies listed in constants.py in the models. So in that respect it's better to keep this separate from constants.py?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@bhilbert4 , I have some shelved code that I'm going to add after this all gets merged. The Anomalies aren't done yet, i'm focusing on the header information for this merge (but wanted to at least get the anomalies in the models so we only have to do one migration).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

also! I'm still running a final test, I think the update will take many hours on my machine, and it will re-start if I lose any connectivity (it has happened once already), so there may be a small change or two that will come in if I find anything else
If you prefer I can tag you in a comment when I'm confident all is good

@melanieclarke
Copy link
Collaborator

@BradleySappington - can we please add expstart to the root file as well, since it's used for sorting? I thought obsstart was equivalent, but it isn't.

@BradleySappington
Copy link
Collaborator Author

@melanieclarke Sure can. I'll comment in here and remove the WIP tag when its ready for review

@BradleySappington BradleySappington changed the title WIP: Update Models for Filter Fields Update Models for Filter Fields Mar 6, 2023
@BradleySappington
Copy link
Collaborator Author

@melanieclarke @bhilbert4 @mfixstsci - Ready for review.
Tested Successfully, jwql-test has up to date fits header information.

Copy link
Collaborator

@melanieclarke melanieclarke left a comment

Choose a reason for hiding this comment

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

This looks good!

I had some trouble getting django to pick up the migrations correctly in my local install. Since there wasn't anything of value in my local database, I just removed it and redid the migrations from the start. I am recreating the database now with archive_database_update and the new tables look good. I'll test some of my pending work against it when the update is complete.

One general question -- will the anomalies model replace the postgres table or be updated from it? And if updated, will it be by cron job or instantaneously when users make new selections? That class of data is a little different from the file info, since updates are made dynamically by JWQL users -- syncing sounds a little more challenging in this case.

jwql/website/apps/jwql/data_containers.py Show resolved Hide resolved
@BradleySappington
Copy link
Collaborator Author

BradleySappington commented Mar 6, 2023

One general question -- will the anomalies model replace the postgres table or be updated from it? And if updated, will it be by cron job or instantaneously when users make new selections? That class of data is a little different from the file info, since updates are made dynamically by JWQL users -- syncing sounds a little more challenging in this case.

@melanieclarke
My plan is to create a script to migrate the postgres table into the django models. we would then update the django models directly from the submit anomaly form. and all anomaly information would be read direclty from the models. The postgres db would no longer be needed at all.

@BradleySappington
Copy link
Collaborator Author

When this gets merged all affected servers will need to:

  1. run makemigrations
  2. run migrate
  3. temporarily comment out their cronjob that runs archive_database_update.py
  4. run $python archive_database_update.py update
  5. when complete re-enable removed cronjob

@melanieclarke
Copy link
Collaborator

My plan is to create a script to migrate the postgres table into the django models. we would then update the django models directly from the submit anomaly form. and all anomaly information would be read direclty from the models. The postgres db would no longer be needed at all.

Excellent, that sounds much cleaner.

Copy link
Collaborator

@melanieclarke melanieclarke left a comment

Choose a reason for hiding this comment

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

Everything looks good to me in local tests. It's taking a while to update my local database, but new tables look great for the instruments that have completed and I was able to find all the new fields I needed for the report download work.

@BradleySappington
Copy link
Collaborator Author

@melanieclarke The "update" flag will make this all take a very long time. Fortunately it shouldn't be too bad on our daily cronjobs because that flag will be omitted.

@mfixstsci mfixstsci merged commit 15e6e78 into spacetelescope:develop Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants