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

TDSv71 and other fixes #5269

Merged
merged 24 commits into from
Mar 2, 2022
Merged

TDSv71 and other fixes #5269

merged 24 commits into from
Mar 2, 2022

Conversation

mattjdnv
Copy link
Contributor

Re-enable TDSv71 and various translation fixes

@brianhatchl I modified some of the NodeJs translation tests and added TDSv71 ones

@@ -33,7 +33,7 @@ hoot.require('SchemaTools');
hoot.require('tds70');
hoot.require('tds70_schema');
hoot.require('tds70_rules');
// hoot.require('config');
hoot.require('config');
Copy link
Contributor

Choose a reason for hiding this comment

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

I see other scripts removing this commented out line. Is it really needed here? or should it be removed too.

Copy link
Contributor Author

@mattjdnv mattjdnv Feb 28, 2022

Choose a reason for hiding this comment

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

Removed along with some other commented out config.xxx lines

Copy link
Contributor

@brianhatchl brianhatchl left a comment

Choose a reason for hiding this comment

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

Yes, lots of diffs to the translation tests, but is any translation behavior really changing?

@mattjdnv
Copy link
Contributor Author

The main behavior changes:

  • Added TDSv71 tests
  • Converted some of the tests into loops checking each schema
  • Broke some tests out into their own files instead of being part of TranslationServer.js

@mattjdnv
Copy link
Contributor Author

mattjdnv commented Feb 28, 2022

I'm still trying to figure out why CircleCI is failing during make archive. I'm not sure if it is a timeout issue.
The weird thing is I changed the order in Makefile.hoot and a different schema build failed!

It seems to be the last one in the list each time:

/bin/bash: line 1:  3786 Killed                  scripts/schema/ConvertTDSv71Schema_XML.py --fullschema conf/translations/TDSv71.xml.gz > translations/tds71_full_schema.js
make[1]: *** [translations/tds71_full_schema.js] Error 255
make[1]: *** Waiting for unfinished jobs...

@mattjdnv
Copy link
Contributor Author

mattjdnv commented Mar 1, 2022

After ssh-ing into Circle CI, I think it is running out of resources when trying to run the schema generation in parallel.
I have added a 'make' target that the ci archive script can call to build the schema before trying to build the rest. I have also added a make job limit when the ci archive runs the target to try and stop it running out of resources.

A better option may be to just commit the generated schema files.

@mattjdnv
Copy link
Contributor Author

mattjdnv commented Mar 1, 2022

Yes!!!
CircleCI is happy again.

Copy link
Contributor

@brianhatchl brianhatchl left a comment

Choose a reason for hiding this comment

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

Nice 😎

mattjdnv added 2 commits March 1, 2022 09:31
NOTE: These are still valid targets in the Makefile.hoot so that they can be updated if needed

refs: #5260
@mattjdnv
Copy link
Contributor Author

mattjdnv commented Mar 1, 2022

Need to do some final QA then can merge.

@mattjdnv
Copy link
Contributor Author

mattjdnv commented Mar 2, 2022

Fixes:

  • Fixed the default values in the TDSv71 Thematic Schema
  • Fixed how the FCSUBTYPE attribute is populated when writing to a thematic layer
  • Broke out a couple of tests from the TranslationServer into their own files.

@mattjdnv mattjdnv merged commit 207ace3 into master Mar 2, 2022
@mattjdnv mattjdnv deleted the 5260 branch March 2, 2022 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants