You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Per the docs, the watch command should only ever deploy modified files, however in some cases, it will deploy files that have not changed locally on disk.
As mentioned in #128, there is a current issue where watch will deploy any file in the same directory as a modified file that starts with the same character as the modified file. However, even with that issue addressed (see #137), there is another issue in the logic that determines which files to deploy when a file is modified. This will occur in either of the following situations:
there are multiple files in the same directory as the modified file that "start with" the same characters (e.g., modifying the page bill.xml will result in both bill.xml and billy.xml being deployed).
Modifying any file in the "site" directory will result in all files in "sitepermissionsets" being deployed
Both #128 and the issue described here are critical flaws in the way watch behaves and could lead to unexpected results for which the user isn't even aware that a file(s) was deployed as standard usage of skuid-cli doesn't emit the filenames deployed when changes are detected. In short, the user would have to have diagnostic and/or trace enabled AND be paying very close attention to the logs emitted to even be aware that unmodified files were deployed.
For example, if the files locally on disk are stale/old and a file is modified intentionally, if any of the old files are deployed, it could overwrite more recent updates to the files/metadata in the target site - all potentially unbeknownst to the individual modifying the file AND potentially against a production system (if that is the target site).
Expected Result
In theory, only the file site/site.json should be deployed but if necessary for Skuid to deploy properly, at most, all the files in site/ directory are deployed.
Actual Result
All files in site/ and sitepermissionsets/ are deployed
The issue is occurring because of how the value for changedEntity is determined and the way ArchivePartial behaves. In short, the approach uses the base name of the file (not the full filename) and performs a starts with (prefix) compare against all files on disk (in the case of "site" files, the word "site" is used as the prefix and hence why "sitepermissionsets" are also deployed). This results in false positives being detected when files start with the same characters (e.g. bill & billy, site & sitepermissionsets). I believe it takes this approach to ensure that related/associated metadata files for the modified file are also deployed (e.g. pages/bill.xml & pages/bill.json both need to deploy whenever one or the other is modified), however it has the negative consequence of deploying files that have not changed.
Note that resolution of the base name of the file itself isn't even foolproof as it simply takes anything to the left of the first period (.) as the base name. For situations where an entity has a period in its name (e.g., files/my.javascript.file.js) this would result in anything matching files/my* getting deployed and a reason to move away from "starts with" approach.
PR fix: resolve several issues with watch command #137 addresses the issues in bug: watch deploys all files in modified file directory that start with same character as modified file #128 but does not address the problems described in this issue as it would require a much more invasive approach to resolve. Based on what I'm seeing in the code, rather than "starts with" detection, there likely needs to be detection of the "metadata type" and then specific logic against the full filename of the modified file to include any associated/related files that must be included in the payload for the modified file (e.g. pages/bill.xml would be detected as a page and then pages/bill.json specifically included because its known a page has a json file). As there isn't any public documentation of all the different metadata types and files associated with each "entity" and without a full test suite, this is best left for internal Skuid team to identify best approach and address. With that said and as mentioned above, I believe this should be considered a critical bug which could lead to completely unexpected behavior against a potentially production system without the user even knowing it and therefore should be addressed ASAP.
The text was updated successfully, but these errors were encountered:
Resolvesskuid#138
Addresses the fatal termination portion of 141 & 142
techfg
changed the title
watch command deploys unmodified files that match a modified files filename
bug: watch command deploys unmodified files that match a modified files filename
Jul 23, 2024
Per the docs, the
watch
command should only ever deploy modified files, however in some cases, it will deploy files that have not changed locally on disk.As mentioned in #128, there is a current issue where
watch
will deploy any file in the same directory as a modified file that starts with the same character as the modified file. However, even with that issue addressed (see #137), there is another issue in the logic that determines which files to deploy when a file is modified. This will occur in either of the following situations:bill.xml
will result in bothbill.xml
andbilly.xml
being deployed).Both #128 and the issue described here are critical flaws in the way
watch
behaves and could lead to unexpected results for which the user isn't even aware that a file(s) was deployed as standard usage ofskuid-cli
doesn't emit the filenames deployed when changes are detected. In short, the user would have to have diagnostic and/or trace enabled AND be paying very close attention to the logs emitted to even be aware that unmodified files were deployed.For example, if the files locally on disk are stale/old and a file is modified intentionally, if any of the old files are deployed, it could overwrite more recent updates to the files/metadata in the target site - all potentially unbeknownst to the individual modifying the file AND potentially against a production system (if that is the target site).
Steps to reproduce
watch
deploys all files in modified file directory that start with same character as modified file #128 so that the issue described here is more apparentfix/resolve-watch-issues
branchgo build
bill
,billy
(no need to put anything in the page)skuid retrieve -d skuid-objects
using the binary from step 3skuid watch -d skuid-objects --trace
using the binary from step 3pages/bill.xml
and save the fileExpected Result
Only
pages/bill.xml
and its correspondingpages/bill.json
should be deployedActual Result
The xml & json files for BOTH
bill
andbilly
in thepages
directory are all deployed.Relevant Log Entries
pages/bill
in the first line as expectedSteps to reproduce (continued)
site/site.json
and save the fileExpected Result
In theory, only the file
site/site.json
should be deployed but if necessary for Skuid to deploy properly, at most, all the files insite/
directory are deployed.Actual Result
All files in
site/
andsitepermissionsets/
are deployedRelevant Log Entries
Additional Information
base name
of the file (not the full filename) and performs astarts with
(prefix) compare against all files on disk (in the case of "site" files, the word "site" is used as the prefix and hence why "sitepermissionsets" are also deployed). This results in false positives being detected when files start with the same characters (e.g. bill & billy, site & sitepermissionsets). I believe it takes this approach to ensure that related/associated metadata files for the modified file are also deployed (e.g. pages/bill.xml & pages/bill.json both need to deploy whenever one or the other is modified), however it has the negative consequence of deploying files that have not changed.base name
of the file itself isn't even foolproof as it simply takes anything to the left of the first period (.
) as the base name. For situations where an entity has a period in its name (e.g., files/my.javascript.file.js) this would result in anything matching files/my* getting deployed and a reason to move away from "starts with" approach.watch
deploys all files in modified file directory that start with same character as modified file #128 but does not address the problems described in this issue as it would require a much more invasive approach to resolve. Based on what I'm seeing in the code, rather than "starts with" detection, there likely needs to be detection of the "metadata type" and then specific logic against the full filename of the modified file to include any associated/related files that must be included in the payload for the modified file (e.g. pages/bill.xml would be detected as apage
and thenpages/bill.json
specifically included because its known a page has a json file). As there isn't any public documentation of all the different metadata types and files associated with each "entity" and without a full test suite, this is best left for internal Skuid team to identify best approach and address. With that said and as mentioned above, I believe this should be considered a critical bug which could lead to completely unexpected behavior against a potentially production system without the user even knowing it and therefore should be addressed ASAP.The text was updated successfully, but these errors were encountered: