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 helpText to requires #373

Merged

Conversation

Falke-Design
Copy link
Contributor

Add helpText to layer.requires like discussed in openmaptiles/openmaptiles#1220

I had to change from SELECT 'osm_ocean_polygon'::regclass; to PERFORM 'osm_ocean_polygon'::regclass; because plpgsql needs a return value if SELECT is used.

Also added an error if a function has no arguments:

when invalid_text_representation then
        RAISE EXCEPTION '%! The arguments of the required function "osmFunc" of the layer "water" are missing. Example: "osmFunc(text)"', SQLERRM;

Example:

layer:
  id: "water"
  requires:
    helpText: 'This is a text with "quotes"'
    functions: "osmFunc"
    tables: "osm_ocean_polygon"

SQL:

-- Assert osm_ocean_polygon exists
do $$
begin
        PERFORM 'osm_ocean_polygon'::regclass;
exception when undefined_table then
        RAISE EXCEPTION '%! This is error text with "quotes"', SQLERRM;
end;
$$ language 'plpgsql';

-- Assert osmFunc exists
do $$
begin
        PERFORM 'osmFunc'::regprocedure;
exception when undefined_function then
        RAISE EXCEPTION '%! This is a text with "quotes"', SQLERRM;
when invalid_text_representation then
        RAISE EXCEPTION '%! The arguments of the required function "osmFunc" of the layer "water" are missing. Example: "osmFunc(text)"', SQLERRM;
end;
$$ language 'plpgsql';

Result of make import-sql:

psql:/sql/parallel/water__waterway.sql:10: ERROR: relation "osm_ocean_polygon2" does not exist! This is a text with "quotes"

@Falke-Design
Copy link
Contributor Author

@nyurik thanks for cleaning up! I didn't used DETAIL or HINT because now the Exception Message is not anymore in one line and can't be catched anymore as "full stacktrace" and printed again at the end.

DO
Time: 0.871 ms
psql:/sql/parallel/water__waterway.sql:14: ERROR:  expected a left parenthesis
DETAIL:  Required function "osmFunc" in layer "water" is incorrectly declared. Use full function signature with parameter types, e.g. "my_magic_func(TEXT, TEXT)"
CONTEXT:  PL/pgSQL function inline_code_block line 9 at RAISE
Time: 0.486 ms
xargs: sh: exited with status 255; aborting

*** ERROR detected, aborting:
psql:/sql/parallel/water__waterway.sql:14: ERROR:  expected a left parenthesis
make: *** [Makefile:422: import-sql] Error 1

If you want to throw the Exception with DETAIL then I suggest that I update the PR openmaptiles/openmaptiles#1237
Then it should be like: If ERROR found, print ERROR Line. If ERROR and then in the next lines DETAIL found, print DETAIL

@nyurik
Copy link
Member

nyurik commented Sep 21, 2021

Oh, i didn't think about that... do you think it would be possible to print last N surrounding lines instead of just the one line with awk / grep ? Also, note that it is not just ERROR and DETAIL, it could also be HINT.

We could of course make it into a single line, but it won't be as readable i think?

@Falke-Design
Copy link
Contributor Author

I found an way, but now the DETAIL text is printed before the ERROR text:
grafik

awk '1{print; fflush()} $$0~".*ERROR" {err[NR]=$$0; errFound=1;} $$0~".*DETAIL" { if(errFound == 1){err[NR]=$$0;} }  END{ if(length(err) > 0){print "\n*** ERROR detected, aborting:"; for(i in err){print err[i]}; exit(1)} }'

Simplified explanation:

  • Check if DETAIL or ERROR is found in line
  • if ERROR found: line is added to the key-value array err. The key is the line number NR err[NR]=$$0. And the flag errFound is set to 1
  • if DETAIL found and flag errFound is 1: line is added to the key-value array err.
  • At the end of the output it loops through the error array.

@nyurik
Copy link
Member

nyurik commented Sep 21, 2021

I suspect it will be far simpler to edit the import-sql -- and do all the error detection there. This way we can even capture things with tee and grep the output to just the lines around the error (for example)

@nyurik
Copy link
Member

nyurik commented Sep 21, 2021

P.S. i think once we update the import-sql script, we can release a new tools version - long overdue. Let me know how you want to proceed with this.

@Falke-Design
Copy link
Contributor Author

It would be much cleaner to do this in import-sql but that was not possible for me. Because of the parallel processing with find and xarg I was not able to catch the errors and then print the errors again.
When a error happens while a sql process, it "informs" xarg to stop all other parallel processes, xarg run exit 255. After this exit I was not able to run code to print the error again.

The result was that i switched to the Makefile with awk

@Falke-Design
Copy link
Contributor Author

To the release:

if it is ok for you i will add in the next 2 days a documention about #361 to the /docs.

And I have to test again if this error still happens: #363 (maybe you can test it too)

@nyurik
Copy link
Member

nyurik commented Sep 28, 2021

I think I will do #376 - rewrite import-sql in python, which will have proper error output

@TomPohys
Copy link
Member

TomPohys commented Oct 8, 2021

Thanks! It looks great!

@TomPohys TomPohys merged commit 208dd6a into openmaptiles:master Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants