Output of Schema with periods (dots) in name breaks SC Ad #130

CraigRegester · 2022-02-17T18:57:22Z

(latest commit testing)

When adding a Schema to source control (SC Menu->Add), receive error in browser:

Looking in WebUI afterwards, I can see it partially worked but clearly does not like the dots - note if I click either of the items in this screenshot, I see nothing.

Maybe has something to do with the output pathing I desire to use (which you can see on 2nd screenshot.)

isc-svelury · 2022-02-17T19:13:19Z

If the two files are new files, then the workspace doesn't display anything. This is a known bug. I created an issue with all the known UI bugs #131 .

CraigRegester · 2022-02-17T19:21:48Z

So that's the 2nd part of the issue then but seems like the first part (where it's referencing the new global for tracking) is different and also after it outputs on the filesystem... it's creating folders w/ the dots still.

Output in the filesystem:

In_Cdi_Rslt_2/3:
total 8
drwxrwsr-x    2 cacheusr gitusers        256 Feb 17 13:50 .
drwxrwsr-x    3 cacheusr gitusers        256 Feb 17 13:50 ..
-rwxrw-r--    1 cacheusr gitusers        339 Feb 17 13:50 1.hl7

(Actual schema name - when viewed in studio - is In_Cdi_Rslt_2.3.1.hl7)

isc-svelury · 2022-02-17T19:58:08Z

From my understanding it looks like the file isn't being exported as expected and so the InternalName is coming up as an empty string. I think that's where we get the subscript error is from.

How are your mappings configured? I am not able replicate the issue. I switched the mapping of .inc files to NoFolders and adding and removing from source control seems to work as expected for me. The flat folder structure is maintained even when I add multiple .inc files from different packages.

CraigRegester · 2022-02-17T20:48:17Z

Yea so if I turn on 'NoFolders', it "works" to a degree (though I still got the subscript error):

Wonder if I need to flush out git-source-control and reload (as I tried to just load the latest repo code using zpm without purging out the previous stuff... so did it initialize anything you need to track the globals properly after switching from sclist?

It does output "right" filename wise with NoFolders

cacheusr@oairsint1:/hfs/sys/git/IRISHealth/hl7$ ll
total 16
drwxrwsr-x    3 cacheusr gitusers        256 Feb 17 15:39 .
drwxrwsr-x    6 cacheusr gitusers        256 Feb 17 13:40 ..
-rwxrw-r--    1 cacheusr gitusers        640 Feb 17 15:36 HealthShare_2.5.hl7
-rwxrw-r--    1 cacheusr gitusers        400 Feb 17 15:39 In_Ep2_Rslt_2.5.1.hl7

Though it's not foldering where I need it to address the multiple namespace factor so I may need to dig in a bit there with a fork to see if I can get it to respect similar foldering that works in other areas (e.g., DTLs, CLS, etc.)

(INTGBUS being an example of the namespace 'folder') Gonna try to flush out what zpm currently has installed/loaded and start fresh first.

isc-tleavitt · 2022-02-17T20:51:57Z

Really the issue here is in the settings UI - you can still define a folder with "NoFolders" (which is re: the dot behavior in the filename).

isc-tleavitt · 2022-02-17T20:57:46Z

Workaround re: file/folder naming will be:

set ^SYS("SourceControl","Git","settings","mappings","HL7","*")="INGTBUS/hl7/"
set ^SYS("SourceControl","Git","settings","mappings","HL7","*","NoFolders")=1

Still need to sort out the error via defensive coding and testing with HL7 schemas specifically to see if (perhaps) the %Studio.AbstractDocument type is throwing things off.

CraigRegester · 2022-02-17T20:59:17Z

Ah interesting, so you're saying it shouldn't* (EDIT) be blocking me out of that last column when I enable NoFolders, gotcha. Yea wondered why that was but was foggy on the idea of 'NoFolders' so wasnt sure.

CraigRegester · 2022-02-18T17:08:30Z

Turned off SC in MP, flushed out git-source-control and re-loaded latest code. All re-initialized (also reset my git folder and set it back up - completely clean) Still seeing this when doing an 'Add'... *

But! Using @isc-tleavitt workaround above, the file does output right now!

CraigRegester · 2022-02-22T23:01:25Z

Are you guys able to reproduce what I'm seeing with the Add command inside MP for LUTs/Schemas?

Wasn't sure if this was specifically happening just to me or a known bug. I see @isc-svelury mentioned it looked like it wasn't getting the item name which I concur.

Will dig more tmrw myself.

isc-svelury · 2022-02-23T14:56:54Z

I haven't gotten around to reproducing it yet. I'll keep you posted.

CraigRegester · 2022-02-24T16:02:05Z

I haven't gotten around to reproducing it yet. I'll keep you posted.

No worries, just making sure I didnt need to provide more info! Sounds like you guys figured it out though so I'll stop digging. Ty both!

isc-svelury · 2022-03-11T21:35:32Z

@CraigRegester as an update, I replicated the issue you were seeing. You were seeing a subscript error because NameToInternalName() returns an empty string for files that aren't in a typical export format even if there are mappings for that file type. This is the specific snippet:

git-source-control/cls/SourceControl/Git/Utils.cls

Lines 1558 to 1566 in 9068eb3

    
                   set fileExt=$zconvert($piece(nam,".",$length(nam,".")),"L") 
        
                   if (InternalName="")&&('$data(ext)=0)&&('$listfind($listbuild("xml","rtn"),fileExt)) { 
        
                       //no match found yet, and this is in a subdir for a specific document type (ext), however it is not in a typical export format 
        
                       //so treat it as a non-mapped file 
        
                       kill ext 
        
                   } 
        
                   if $data(ext)=0 { 
        
                       quit "" 
        
                   }

I have fixed the issue and also fixed the mappings in the settings page. I still have some loose ends to wrap up and then I'll send a PR to close this issue.

CraigRegester · 2022-03-11T21:37:44Z

Wonderful! ty for the update, Glad you were able to reproduce. I haven't had time to dig in myself yet. Once PR is done, I'll pull latest commits again and retest. Excited to get using it!

CraigRegester · 2022-03-21T14:36:51Z

Just a heads up, this issue (or the original related one that led to NoFolders) impacts the VS Code objectscript extension as well.

When using non-ISFS mode and choosing to export an HL7 schema with dots in the name (even the base schemas are like this), the dots are converted to slashes/folders on export.

Wonder if any of the efforts you guys are making here can help address it over there as well.

isc-tleavitt · 2022-03-21T14:42:34Z

@CraigRegester ultimately the two issues are disconnected; we'll need a separate solution in VSCode export. Do you have a related VSCode issue?

CraigRegester · 2022-03-21T14:44:05Z

I do not but can open one. Just wasn't sure how connected your group was to the VSCode one. No worries!

isc-tleavitt · 2022-03-21T14:45:08Z

Not super connected, but we know who to pester and I've occasionally contributed to the vscode extension. ;)

CraigRegester · 2022-03-29T13:28:17Z

Pulled latest commits and re-tested... I still get this when I go to Lookup Tables and try to add a table with the source control menu:

However, when I look in the output, it seems to at least partially be working now? Maybe more I can check to help figure out what's causing the pop up error?

CraigRegester · 2022-03-29T13:29:16Z

See this in the Git GUI

isc-svelury · 2022-03-29T13:43:31Z

How are your mappings set up? Could you add a screenshot of the output from zw ^SYS("SourceControl","Git","settings","mappings")?

The 'Could not access' error is happening because the highlighted entry is a directory. It's simple to expand the directory and list its files. I opened #175 for it. I'll send a PR against that.

CraigRegester · 2022-03-29T14:57:10Z

Yea I tried resetting my mappings after my comment above as I realized that might be part of it.... so here they are though I'm getting a different error now.... screenshot after the mappings:

^SYS("SourceControl","Git","settings","mappings","CLS","*")="INTGBUS/src/cls/"
^SYS("SourceControl","Git","settings","mappings","CLS","UnitTest")="INTGBUS/src/cls/UnitTest/"
^SYS("SourceControl","Git","settings","mappings","HL7","*")="INTGBUS/src/oth/"
^SYS("SourceControl","Git","settings","mappings","HL7","*","NoFolders")=1
^SYS("SourceControl","Git","settings","mappings","INC","*")="INTGBUS/src/inc/"
^SYS("SourceControl","Git","settings","mappings","LUT","*")="INTGBUS/src/oth/"
^SYS("SourceControl","Git","settings","mappings","MAC","*")="INTGBUS/src/mac/"

Nothing in the output window with that error. So just failed out. BUT - if I add a value to the table and click Save, it appears to work:

So maybe just the Add workflow is broken.

isc-svelury · 2022-03-29T15:32:57Z

I tried to replicate this issue and I did see something similar when the LUT file I created wasn't saved before adding to source control. In that case, the export of the item failed because the file did not exist.

Can you try:

set sc=##class(SourceControl.Git.Utils).ExportItem("<filename>",,1,.filenames)
w $System.Status.GetErrorText(sc)

If it's the same issue you should see something like ERROR #16005: Document '<filename>' does NOT exist.

I didn't run into any errors other than that.

CraigRegester · 2022-03-29T15:41:55Z

So I think I wasn't clear on my earlier comments - these tables already existed. They aren't new tables. In the first case, I just went into an existing table and went to 'Add' it to source control and got the SUBSCRIPT error. In the second case, I fixed my mappings so it could see the file already out on the filesystem in my repo but that triggered the UNDEFINED error for the filenames variable.

Here's the output of your command provided - I tried it 3 times, 1 on a table that already exists and exists in the Globals, 1 on a table that already exists but hadn't be added to the globals tracker yet, and 1 on a table that doesn't exist at all.... only in the latter case did I get the error you expected (which makes sense - it doesn't exist! but that's not the issue I was having earlier)

DEV:cacheusr:INTGBUS>set sc=##class(SourceControl.Git.Utils).ExportItem("TestTable.lut", ,1,.filenames)
exporting new version of TestTable.lut to /hfs/sys/git/ISC-IRISHealth/INTGBUS/src/oth/TestTable.lut

DEV:cacheusr:INTGBUS>w $System.Status.GetErrorText(sc)

DEV:cacheusr:INTGBUS>set sc=##class(SourceControl.Git.Utils).ExportItem("TestTable2.lut", ,1,.filenames)
exporting new version of TestTable2.lut to /hfs/sys/git/ISC-IRISHealth/INTGBUS/src/oth/TestTable2.lut

DEV:cacheusr:INTGBUS>set sc=##class(SourceControl.Git.Utils).ExportItem("TestTable3.lut", ,1,.filenames)
exporting new version of TestTable3.lut to /hfs/sys/git/ISC-IRISHealth/INTGBUS/src/oth/TestTable3.lut

DEV:cacheusr:INTGBUS>w $System.Status.GetErrorText(sc)
ERROR #16005: Document 'TestTable3.lut' does NOT exist

CraigRegester · 2022-03-29T15:50:12Z

Additional interesting note... this command on an existing table that was not yet present in the Global tracker:

DEV:cacheusr:INTGBUS>set sc=##class(SourceControl.Git.Utils).ExportItem("TestTable2.lut", ,1,.filenames)
exporting new version of TestTable2.lut to /hfs/sys/git/ISC-IRISHealth/INTGBUS/src/oth/TestTable2.lut

It created a global entry for TSH (timestamp of last compile) but not one for items...

`
zw ^SYS("SourceControl","Git","items","TestTable.lut")
^SYS("SourceControl","Git","items","TestTable.lut")=""
zw ^SYS("SourceControl","Git","TSH","TestTable.lut")
^SYS("SourceControl","Git","TSH","TestTable.lut")="66197,41839"
^-- Good
zw ^SYS("SourceControl","Git","TSH","TestTable2.lut")
^SYS("SourceControl","Git","TSH","TestTable2.lut")="66197,41861"
zw ^SYS("SourceControl","Git","items","TestTable2.lut")

^--- missing?
`

isc-svelury · 2022-03-29T16:06:35Z

So to explain my reasoning for checking if the export works as expected is that the filenames variable is updated while exporting an item. It's mainly used for complex items like packages that can have multiple sub-items associated with them. If we are adding a single file, filenames should have a node corresponding to that file after export.

@isc-tleavitt do you know why filenames might be undefined if export works as expected?

isc-svelury · 2022-03-29T16:09:12Z

Exporting an item just copies the latest version of the file from IRIS into the filesystem according to the mappings (or default behaviour if mappings don't exist). It does not add the item to source control. So ^SYS("SourceControl","Git","items") is not updated in that case.

isc-tleavitt · 2022-03-29T16:35:41Z

@CraigRegester are you sure you have the latest commits loaded?

Would be interested to know if SourceControl.Git.Utils:AddToSourceControl calls ExportItem with the 3rd arg set to 1. (Not doing so could explain the issue - I was going to comment saying so based on the older version I have installed but see that it has changed.)

CraigRegester · 2022-03-29T16:46:44Z

Will double check that particular example but yes, cloned into a fresh directory yesterday and did a load using zpm

CraigRegester · 2022-03-29T17:27:23Z

@isc-tleavitt You are correct that the method is out of date.... but why? If I look at the package I cloned yesterday, it is correct... has the 1 in the third parameter. So why when I did a zpm load from that directory did it not refresh all the classes?

Is there a parameter I need to toss on the zpm load?

isc-tleavitt · 2022-03-29T17:31:44Z

@CraigRegester it should just be zpm "load /path/to/where/you/cloned" - maybe you picked the wrong directory?

CraigRegester · 2022-03-29T17:47:44Z

Definitely the right directory as I deleted the old one to be safe before I cloned... hmmm...

Looks like I ran the load into %SYS originally and most recently even - but it is a different namespace making use of it. So let me load into that namespace instead. Maybe my misunderstanding of ZPM but I thought by loading in %SYS, the package (SourceControl in this case) was just mapped everywhere through a %ALL kinda method.

CraigRegester · 2022-03-29T17:50:02Z

That's what it was - ugh, sorry, embarrassed. I blame my novice use of ZPM

Will retest now (though I will note I was able to see the updated Web UI and such just by loading into %SYS so that definitely threw me off the scent!)

isc-tleavitt · 2022-03-29T17:53:54Z

@CraigRegester an understandable misunderstanding - some packages do set up a %ALL mapping. I personally don't like doing that because it's a bit more invasive than necessary, but perhaps it would make sense to do that with git-source-control - or at least offering the option to do so.

CraigRegester · 2022-03-29T17:56:25Z

No problem - I have no strong opinion other than maybe a call out when pulling down new versions to be sure to do it in the proper namespace and/or each namespace (if multiple namespaces on that instance.)

Definitely working better now! Only weird thing that popped up on me is after adding a table, I asked it to discard changes and got this:

But that may still be a work in progress.

isc-svelury · 2022-03-29T18:06:56Z

I see that too. I think we're seeing this error because the method used to load files does support loading .lut files. I think we will need to export the file out as an XML file and it should work. I created an issue for that: #176

isc-tleavitt assigned isc-svelury Feb 17, 2022

isc-tleavitt added the bug Something isn't working label Feb 24, 2022

isc-tleavitt added this to the v2.0.0 milestone Feb 24, 2022

isc-svelury mentioned this issue Feb 25, 2022

Minor bug fixes in utils #136

Merged

isc-svelury mentioned this issue Mar 14, 2022

No folders fix #138

Merged

isc-tleavitt closed this as completed in #138 Mar 22, 2022

isc-svelury mentioned this issue Mar 23, 2022

NoFolders check for mappings in ListItemsInFiles() #167

Closed

isc-svelury mentioned this issue Mar 29, 2022

Diff does not work with untracked directories in the Staging Area of the Workspace #175

Closed

Output of Schema with periods (dots) in name breaks SC Ad #130

Output of Schema with periods (dots) in name breaks SC Ad #130

Comments

CraigRegester commented Feb 17, 2022

isc-svelury commented Feb 17, 2022

CraigRegester commented Feb 17, 2022 • edited Loading

isc-svelury commented Feb 17, 2022 • edited Loading

CraigRegester commented Feb 17, 2022

isc-tleavitt commented Feb 17, 2022

isc-tleavitt commented Feb 17, 2022

CraigRegester commented Feb 17, 2022 • edited Loading

CraigRegester commented Feb 18, 2022

CraigRegester commented Feb 22, 2022

isc-svelury commented Feb 23, 2022

CraigRegester commented Feb 24, 2022

isc-svelury commented Mar 11, 2022 • edited Loading

CraigRegester commented Mar 11, 2022

CraigRegester commented Mar 21, 2022

isc-tleavitt commented Mar 21, 2022

CraigRegester commented Mar 21, 2022

isc-tleavitt commented Mar 21, 2022

CraigRegester commented Mar 29, 2022

CraigRegester commented Mar 29, 2022

isc-svelury commented Mar 29, 2022

CraigRegester commented Mar 29, 2022

isc-svelury commented Mar 29, 2022 • edited Loading

CraigRegester commented Mar 29, 2022

CraigRegester commented Mar 29, 2022

isc-svelury commented Mar 29, 2022

isc-svelury commented Mar 29, 2022

isc-tleavitt commented Mar 29, 2022

CraigRegester commented Mar 29, 2022

CraigRegester commented Mar 29, 2022 • edited Loading

isc-tleavitt commented Mar 29, 2022

CraigRegester commented Mar 29, 2022

CraigRegester commented Mar 29, 2022 • edited Loading

isc-tleavitt commented Mar 29, 2022

CraigRegester commented Mar 29, 2022

isc-svelury commented Mar 29, 2022

CraigRegester commented Feb 17, 2022 •

edited

Loading

isc-svelury commented Feb 17, 2022 •

edited

Loading

CraigRegester commented Feb 17, 2022 •

edited

Loading

isc-svelury commented Mar 11, 2022 •

edited

Loading

isc-svelury commented Mar 29, 2022 •

edited

Loading

CraigRegester commented Mar 29, 2022 •

edited

Loading

CraigRegester commented Mar 29, 2022 •

edited

Loading