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

Add Specify 6.8.03 Static Files to Specify 7 #5266

Merged
merged 42 commits into from
Oct 28, 2024
Merged

Conversation

acwhite211
Copy link
Member

@acwhite211 acwhite211 commented Sep 10, 2024

Fixes #4828
Fixes #5258

Copy the config directory from Specify 6 into Specify 7 so that the Specify 6 docker container doesn't need to be spun up with the Specify 7 container. Also, edit the THICK_CLIENT_LOCATION in the specify settings so that the SPECIFY_CONFIG_DIR gets set to the new path. The nginx configuration file also needs to be edited to the new config directory path.

This is simply moving the default static files from Specify 6 to Specify 7. This will not override custom client specific configs that have been added to the database, since Specify 7 first looks in the database and then in the filesystem:

def get_app_resource(collection, user, resource_name):
"""Fetch the named app resource in the context of the given user and collection.
Returns the resource data and mimetype as a pair.
"""
logger.info('looking for app resource %r for user %s in %s',
resource_name, user and user.name, collection and collection.collectionname)
# Traverse the hierarchy.
for level in DIR_LEVELS:
# First look in the database.
from_db = get_app_resource_from_db(collection, user, level, resource_name)
if from_db is not None: return from_db
# If resource was not found, look on the filesystem.
from_fs = load_resource_at_level(collection, user, level, resource_name)
if from_fs is not None: return from_fs
# Continue to next higher level of hierarchy.
# resource was not found
return None

Note for this Draft PR, not all the files in the Specify 6 config directory are needed. So identifying the files that are not actually ever used by Specify 7 and then pruning those from the Specify 7 config directory still needs to be done as of the creation of this Draft PR.

If we only had to worry about hosted instances, it would be nice just to put these files in S3. Another option to explore, if anyone wants to, would be creating a Django migration to add these default configs as blobs into the database and adding a default field.

Checklist

  • Self-review the PR after opening it to make sure the changes look good
    and self-explanatory (or properly documented)
  • Add automated tests
  • Add relevant issue to release milestone

Testing instructions

TBD

@acwhite211 acwhite211 added this to the 7.9.8 milestone Sep 10, 2024
@acwhite211 acwhite211 self-assigned this Sep 10, 2024
@maxpatiiuk
Copy link
Member

Once you identified and removed the needless static files, make sure to merge this PR with squash so that unused sp6 static files they don't clutter up the specify 7 repository history

Screenshot 2024-09-10 at 16 51 13

@acwhite211
Copy link
Member Author

We'll need to edit the Specify 7 nginx web-server to not depend on the Specify 6 container as well.

Copy link
Member

@maxpatiiuk maxpatiiuk left a comment

Choose a reason for hiding this comment

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

there still appears to be a lot of files in this PR that are not needed by specify 7.

could you search the back-end and front-end for usages, and delete the needless files?
or would you prefer Jason or I look over these?

@acwhite211
Copy link
Member Author

acwhite211 commented Sep 17, 2024

there still appears to be a lot of files in this PR that are not needed by specify 7.

could you search the back-end and front-end for usages, and delete the needless files? or would you prefer Jason or I look over these?

Here are the initial notes I made from going through the code-base, searching file and directory names.

Keep:

accessions
backstop
bird
botany
common
defaultperms
fish
herpetology
invertebrate
invertpaleo
mammal
paleobotany
vertpaleo

Maybe Keep:

insect
reptile
vascplant
fungi
disciplines.xml
icons.xml
icons_datamodel.xml
icons_disciplines.xml
icons_imgproc.xml
icons_plugins.xml
querybuilder.xml
schema_localization.xml
schema_version.xml

Probably Remove:

ireportconfig.xml
lichens_label.jrxml
specify_workbench_upload_def.xml

Remove:

SymbiotaDwcMeta.xml
VertNetCacheMapping.xml
agent_update_wb_datamodel.xml
bubble_info.xml
build_ipad_xreftables.sql
collectingevent_update_wb_datamodel.xml
collectionobject_import_wb_datamodel.xml
collectionobject_update_wb_datamodel.xml
command_registry.xml
darwin_core_map.xml
datamodel_automappings.xml
dwcdefaultmap.xml
es_skipfields.xml
floats_to_decimals.sql
geonames.sql.zip
install_info.xml
mime_icons.xml
new_attch_tables.sql
phonet.en
plugin_registry.xml
prefs_init.xml
locality_update_wb_datamodel.xml
preparation_update_wb_datamodel.xml
rstools_registry.xml
show_fields.xml
specify_tableid_listing.xml
specify_workbench_datamodel.xml
specify_workbench_remappings.xml
synonym_cleanup_en.html
tdwg_dwcterms.xsd
text_factory_mapping.xml
thumbnail_generators.xml
wb_registry.xml
wbschema_localization.xml
ipad_exporter
nameresources
dbdrivers.xml
hiddentables.xml
usertypes.xml

Going through the files in the directories to further filter out files.

@maxpatiiuk
Copy link
Member

Ah, what I mean is that inside each of these folders there are several files that are probably not needed anymore:

accessions
backstop
bird
botany
common
defaultperms
fish
herpetology
invertebrate
invertpaleo
mammal
paleobotany
vertpaleo

dataentry_task.xml, search_config.xml, startup_panel.xml and etc

there also appear to be many reports/labels files - you will need to check with Grant on these but I was under impression that all reports are hand-crafted or done in jasper studio/irepo, so I am not sure what these files are about

@acwhite211
Copy link
Member Author

Note, it looks like the icon xml files are used:

image

@maxpatiiuk
Copy link
Member

@acwhite211 yes, icons are still used, but only for compatibility with sp6 forms.
This PR might be an excellent time to remove that feature.

Sp6 static files include a fixed set of low resolution icons that people can include in their forms.
The icon*.xml files files are mapping the icon names to icon file names for different icon sizes.

Instead of using these legacy icon names, Specify 7 already supports specifying icon image URLs. Those URLs could point to your local asset server, or any external file server.

Thus the workflow could be:

  • less than a dozen of those sp6 icons are included in the default forms - specify 7 could have hardcoded logic to resolve those 10 icons to some file on files.specifysoftware.org
  • going forward, we advice people to put icon URLs instead, and instruct them how to get a URL from an asset server image
  • when we have the visual form editor, there would be a UI option for uploading a new asset or picking an existing asset to get it's URL

Note, this change means that icons that icons display in sp6 won't be displayed in sp7 without some form changes. If that is too much of a breaking change at this point, then we can delay the deprecation till later.
That hover means this PR would need to also include Specify 6 static icons - without those, we are not separated from sp6 static files.

Copy link
Member

@grantfitzsimmons grantfitzsimmons left a comment

Choose a reason for hiding this comment

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

Without any modification, this branch is not working for me.

Initially, when running docker compose up --build, it fails because the specify6 service was defined yet invalid as the specify6 volume was not being created.

  specify6:
    image: specifyconsortium/specify6-service:6.8.03
    volumes:
      - "specify6:/volumes/Specify"

After commenting the above bit out, I try to run it again. Immediately, I see this error:

specify7-1         | FileNotFoundError: [Errno 2] No such file or directory: '/opt/specify7/specify.jar'

I check to see that this is actually missing:

docker-compose run --rm specify7 ls -l /opt/specify7/config

and see

~/GitHub/specify7 on issue-4828 ⇣1 *1 +1 !2 ❯ docker-compose run --rm specify7 ls -l /opt/specify7/config                                                                                            at 01:37:27 PM
WARN[0000] /Users/g584f396/GitHub/specify7/docker-compose.yml: `version` is obsolete
[+] Creating 1/0
 ✔ Container specify7-webpack-1  Running                                                                                                                                                                       0.0s
total 11900
lrwxrwxrwx  1 root root       49 Aug 17 02:57 DataExporter_64.desktop -> .install4j/install4j_15dyqoe-DataExporter.desktop
lrwxrwxrwx  1 root root       55 Aug 17 02:57 ImportFileSplitter_64.desktop -> .install4j/install4j_15dyqoe-ImportFileSplitter.desktop
lrwxrwxrwx  1 root root       52 Aug 17 02:57 SpBackupRestore_64.desktop -> .install4j/install4j_15dyqoe-SpBackupRestore.desktop
lrwxrwxrwx  1 root root       45 Aug 17 02:57 SpWizard_64.desktop -> .install4j/install4j_15dyqoe-SpWizard.desktop
lrwxrwxrwx  1 root root       47 Aug 17 02:57 Specify4GB_64.desktop -> .install4j/install4j_15dyqoe-Specify4GB.desktop
lrwxrwxrwx  1 root root       60 Aug 17 02:57 SpecifyDBSecurityWizard_64.desktop -> .install4j/install4j_15dyqoe-SpecifyDBSecurityWizard.desktop
lrwxrwxrwx  1 root root       44 Aug 17 02:57 Specify_64.desktop -> .install4j/install4j_15dyqoe-Specify.desktop
lrwxrwxrwx  1 root root       46 Aug 17 02:57 SpiReport_64.desktop -> .install4j/install4j_15dyqoe-SpiReport.desktop
drwxr-xr-x  2 root root     4096 Aug 17 02:57 bin
drwxr-xr-x 21 root root     4096 Aug 17 02:57 config
drwxr-xr-x  2 root root     4096 Aug 17 02:57 demo_files
drwxr-xr-x  7 root root     4096 Aug 17 02:57 help
drwxr-xr-x  2 root root     4096 Aug 17 02:57 hibernate_libs
drwxr-xr-x  2 root root     4096 Aug 17 02:57 iReportLibs
drwxr-xr-x  2 root root     4096 Aug 17 02:57 libs
-rw-r--r--  1 root root    15590 Mar  1  2023 license_agreement.html
-rw-r--r--  1 root root     4996 Mar  1  2023 non-mac-application-adapter.jar
drwxr-xr-x  2 root root     4096 Mar  1  2023 plugins
-rw-r--r--  1 root root      446 Mar  1  2023 readme.html
-rw-r--r--  1 root root 12097156 Mar  1  2023 specify.jar
-rwx------  1 root root    14045 Mar  1  2023 uninstall
drwxr-xr-x  2 root root     4096 Aug 17 02:57 wwlibs

So we can verify that specify.jar is accessible within the container at /opt/specify7/config/specify.jar.

Now it seems the SPECIFY_THICK_CLIENT location may be causing the issue. I can fix the issue I see in the specify7-1 container upon startup by changing it in the specify_settings.py file:

# Specify 7 requires the files from a Specify 6 install.
# This setting should point to a directory containing an installation
# of Specify 6 of the same version as the Specify database.
THICK_CLIENT_LOCATION = '/opt/specify7'

Adding /config looks like this:

THICK_CLIENT_LOCATION = '/opt/specify7/config'

Now no startup errors! However, this of course doesn't solve the problem. Now the other files aren't accessible properly, so naturally an error appears:

image

@acwhite211 How have you been testing this? Am I missing something? When you can, please provide testing instructions.

@grantfitzsimmons
Copy link
Member

@maxpatiiuk

@acwhite211 yes, icons are still used, but only for compatibility with sp6 forms.

Icons are also used when thumbnails aren't generated for attachments and for table headings in the WorkBench (until #5176)

This PR might be an excellent time to remove that feature.

I agree that this should be done, but I believe this is not the right time due to other pressing priorities. We must focus our efforts on delivering GeoSpecify on schedule. This should be spun off into a different issue.

@grantfitzsimmons
Copy link
Member

there also appear to be many reports/labels files - you will need to check with Grant on these but I was under impression that all reports are hand-crafted or done in jasper studio/irepo, so I am not sure what these files are about

@acwhite211 There is no need to keep any of the *.jrxml files for Specify 7. @maxpatiiuk is right, these were either hand-crafted or used as defaults in the past. Even in the latest version of Specify 6, you can no longer use these as templates.

Copy link
Member

Choose a reason for hiding this comment

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

while this file is used at the moment, could we refactor back-end to not use app_resources.xml files?

e.g aren't all UIFormatters XML files of the same name, thus removing the need for this file?

it seems that this file is used at present to tell specify what files are defined in a given folder - wouldn't it be better to simply check if the file is present on the file system?

</altviews>
</view>

<view name="WorkbenchTemplate"
Copy link
Member

Choose a reason for hiding this comment

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

should we remove views for sp6-only tables?

Copy link
Member

Choose a reason for hiding this comment

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

this entire file could potentially be removed - need to double check

Copy link
Contributor

@melton-jason melton-jason Oct 8, 2024

Choose a reason for hiding this comment

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

There are a lot of views for tables here that are only defined in this file:

  • Address
  • AgentSpecialty
  • Agent
  • AgentVariant
  • GroupPerson
  • Author
  • Journal
  • ReferenceWork
  • TaxonCitation
  • Taxon
  • Geography
  • Storage
  • GeologicTimePeriod
  • LithoStrat
  • (All of the TreeDefItem forms)
  • ObjectAttachmenmt
  • CollectionObjectAttachment

The other views defined here can likely be removed

</altviews>
</view>

<view name="ChangePassword"
Copy link
Member

Choose a reason for hiding this comment

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

remove.
search for all views that don't have a class="edu.ku.brc.specify.datamodel.* attribute and consider removing them

Copy link
Member

Choose a reason for hiding this comment

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

is it safe to remove schema localization for sp6-only tables or is it too early for that?

@grantfitzsimmons
Copy link
Member

grantfitzsimmons commented Oct 8, 2024

@maxpatiiuk We are finalizing this PR and will address your comments in a future PR. Thank you for your feedback! 😄

I'll open a new issue with your comments soon if you don't beat me to it.

@maxpatiiuk
Copy link
Member

that means all these useless files will end up in the repository's git history forever 😮‍💨

edit: I guess that's ok. Jason already filtered out most of the needless files

@CarolineDenis
Copy link
Contributor

CarolineDenis commented Oct 21, 2024

TODO:
From @grantfitzsimmons:

All of our Specify Cloud instances, self-hosted users, and test panel configurations assume that the Specify 6 Docker image will be the source of the static files that Specify 7 depends on.

@acwhite211
Copy link
Member Author

acwhite211 commented Oct 24, 2024

PR for editing Specify Cloud deployment https://github.com/specify/docker-compositions/pull/32
PR for editing Test Panel deployment specify/specify7-test-panel#122

For the test-panel, the main change is to edit nginx rewrite for config to static-files volume path. The current solution will require all new branches to have config in the Specify 7 container, instead of in a separate Specify 6 container. Further work will be needed in order to support new and old branches where the config location is different for the nginx rewrite path to point to. Need a good way of specifying if a branch uses the new or old config setup. We'll need to add a way to check to see if the branch has the config directory in the Specify 7 filesystem from the dockerCompose.ts file.

@acwhite211 acwhite211 merged commit c9f0342 into production Oct 28, 2024
12 checks passed
@acwhite211 acwhite211 deleted the issue-4828 branch October 28, 2024 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add new geo view def + missing default Add Specify 7 static files system
6 participants