Uploaded image for project: 'IGB'
  1. IGB
  2. IGBF-3774

Fix the order of the data providers being initialized

    Details

    • Type: Task
    • Status: Closed (View Workflow)
    • Priority: Major
    • Resolution: Done
    • Affects Version/s: None
    • Fix Version/s: 10.1.0
    • Labels:
      None

      Description

      Fix the order of the data providers being initialized, right now the data providers are not being initialized based on the load priority but based on the order in the igbDefaultPrefs.json file. Change this behavior by sorting the dataProviders list after retrieving them from the json based on the load priority. This ordering is important to implement Ensembl.

        Attachments

          Issue Links

            Activity

            Hide
            jsirigin Jaya Sravani Sirigineedi added a comment -

            Fixed the order by sorting the list, updated code is available at: https://bitbucket.org/jaya-sravani/integrated-genome-browser/branch/IGBF-3774. Now, the data providers are being initialized based on the load priority, can verify this in the logs. Please review and let me know if there are any issues.

            Show
            jsirigin Jaya Sravani Sirigineedi added a comment - Fixed the order by sorting the list, updated code is available at: https://bitbucket.org/jaya-sravani/integrated-genome-browser/branch/IGBF-3774 . Now, the data providers are being initialized based on the load priority, can verify this in the logs. Please review and let me know if there are any issues.
            Hide
            nfreese Nowlan Freese added a comment -

            Small issue where the UCSC REST provider is being initialized twice. Talked with Sravani and she has a fix, will include as an additional commit.

            Show
            nfreese Nowlan Freese added a comment - Small issue where the UCSC REST provider is being initialized twice. Talked with Sravani and she has a fix, will include as an additional commit.
            Hide
            jsirigin Jaya Sravani Sirigineedi added a comment -

            Fixed it and updated code is present at the branch: https://bitbucket.org/jaya-sravani/integrated-genome-browser/branch/IGBF-3774.

            Show
            jsirigin Jaya Sravani Sirigineedi added a comment - Fixed it and updated code is present at the branch: https://bitbucket.org/jaya-sravani/integrated-genome-browser/branch/IGBF-3774 .
            Hide
            nfreese Nowlan Freese added a comment -

            When building with tests I am seeing test failures in RestApiDataProviderTest. Were the tests dependent on being initialized in RestApiDataProvider.java?

            Show
            nfreese Nowlan Freese added a comment - When building with tests I am seeing test failures in RestApiDataProviderTest. Were the tests dependent on being initialized in RestApiDataProvider.java?
            Hide
            jsirigin Jaya Sravani Sirigineedi added a comment -

            Yes, Dr.Freese they are dependent, I have fixed the issue and updated the commit. I forgot to run the tests in my local, that's why I missed it, thanks for finding it out.

            Show
            jsirigin Jaya Sravani Sirigineedi added a comment - Yes, Dr.Freese they are dependent, I have fixed the issue and updated the commit. I forgot to run the tests in my local, that's why I missed it, thanks for finding it out.
            Hide
            nfreese Nowlan Freese added a comment -

            Tested on Mac using Sravani's branch.

            Data providers are now initialized according to the loadPriority, as indicated by looking at the IGB logs and comparing the initialization order between main and IGBF-3774.

            Note that the loadPriority also appears to determine the order of the Quickloads/REST API in the Available Data window and in the Data Sources tab. See IGBF-1220 for more information. Right now we have several Quickloads set to the same loadPriority, and may want to update these.

            Ready for pull request.

            Show
            nfreese Nowlan Freese added a comment - Tested on Mac using Sravani's branch. Data providers are now initialized according to the loadPriority, as indicated by looking at the IGB logs and comparing the initialization order between main and IGBF-3774 . Note that the loadPriority also appears to determine the order of the Quickloads/REST API in the Available Data window and in the Data Sources tab. See IGBF-1220 for more information. Right now we have several Quickloads set to the same loadPriority, and may want to update these. Ready for pull request.
            Show
            jsirigin Jaya Sravani Sirigineedi added a comment - Created a Pull request: https://bitbucket.org/lorainelab/integrated-genome-browser/pull-requests/1021
            Hide
            ann.loraine Ann Loraine added a comment - - edited

            Before I merge this, I need to ask about whether this change is affecting the order in which Quickload folders appear in the Available Data Sets data set chooser interface.

            How has the folder order changed as a result of this new code?

            A little background:

            "Load priority" got set up in our code as a way to specify the data source that would be relied on to provide genomic sequence data for the genome version being shown. Everything else that it may do is basically a side effect. This has caused problems in the past, and I'm a little worried that due to my memory being incomplete, I'm not remembering some serious "gotchas" that may cause unforeseen problems.

            Also, "load priority" may have been intentionally designed to accommodate ties - e.g., data providers with equal load priority values. The new sorting mechanism may need to handle the case of ties.

            Show
            ann.loraine Ann Loraine added a comment - - edited Before I merge this, I need to ask about whether this change is affecting the order in which Quickload folders appear in the Available Data Sets data set chooser interface. How has the folder order changed as a result of this new code? A little background: "Load priority" got set up in our code as a way to specify the data source that would be relied on to provide genomic sequence data for the genome version being shown. Everything else that it may do is basically a side effect. This has caused problems in the past, and I'm a little worried that due to my memory being incomplete, I'm not remembering some serious "gotchas" that may cause unforeseen problems. Also, "load priority" may have been intentionally designed to accommodate ties - e.g., data providers with equal load priority values. The new sorting mechanism may need to handle the case of ties.
            Hide
            ann.loraine Ann Loraine added a comment -

            Note: I am moving this back to "To Do" with hopes that Jaya Sravani Sirigineedi and Nowlan Freese can take a look.

            Show
            ann.loraine Ann Loraine added a comment - Note: I am moving this back to "To Do" with hopes that Jaya Sravani Sirigineedi and Nowlan Freese can take a look.
            Hide
            nfreese Nowlan Freese added a comment - - edited

            Ann Loraine - Sravani's code change should only affect the order in which the data providers are initialized. She added a sort based on load priority to the IgbPreferencesLoadingOrchestrator class.

            OLD: dataProviders.stream().distinct().forEach(dataProvider
            NEW: dataProviders.stream().distinct().sorted(Comparator.comparing(DataProviderConfig::getLoadPriority)).forEach(dataProvider

            The other code changes in the ticket fix an issue where the UCSC REST data provider was being initialized more than once.

            While I did not specifically test the data loading order, based on comments in IGBF-1220, the loading of data by IGB is also done based on a sorting of the load priority, so I don't foresee the code changes in this ticket affecting it. The order of the Quickload folders is also unaffected, both Sravani and I saw that they continue to follow the load priority.

            Note that we need the order of data provider initialization to be sorted so that we can initialize the Ensembl data provider last. This is important as some of the genomes provided by Ensembl are already included in IGB, and we did not want to have the same genome appearing twice.

            Show
            nfreese Nowlan Freese added a comment - - edited Ann Loraine - Sravani's code change should only affect the order in which the data providers are initialized. She added a sort based on load priority to the IgbPreferencesLoadingOrchestrator class. OLD: dataProviders.stream().distinct().forEach(dataProvider NEW: dataProviders.stream().distinct(). sorted(Comparator.comparing(DataProviderConfig::getLoadPriority)) .forEach(dataProvider The other code changes in the ticket fix an issue where the UCSC REST data provider was being initialized more than once. While I did not specifically test the data loading order, based on comments in IGBF-1220 , the loading of data by IGB is also done based on a sorting of the load priority, so I don't foresee the code changes in this ticket affecting it. The order of the Quickload folders is also unaffected, both Sravani and I saw that they continue to follow the load priority. Note that we need the order of data provider initialization to be sorted so that we can initialize the Ensembl data provider last. This is important as some of the genomes provided by Ensembl are already included in IGB, and we did not want to have the same genome appearing twice.
            Hide
            ann.loraine Ann Loraine added a comment -

            Thanks for the explanation Nowlan Freese! I am proceeding to merge the PR.

            Show
            ann.loraine Ann Loraine added a comment - Thanks for the explanation Nowlan Freese ! I am proceeding to merge the PR.
            Hide
            ann.loraine Ann Loraine added a comment -

            PR is merged and new installers built and deployed to bioviz.org early access section.

            Show
            ann.loraine Ann Loraine added a comment - PR is merged and new installers built and deployed to bioviz.org early access section.
            Hide
            nfreese Nowlan Freese added a comment -

            Tested on Linux with main branch installer.

            Data providers are initialized based on their load priority.

            Closing ticket.

            Show
            nfreese Nowlan Freese added a comment - Tested on Linux with main branch installer. Data providers are initialized based on their load priority. Closing ticket.

              People

              • Assignee:
                jsirigin Jaya Sravani Sirigineedi
                Reporter:
                jsirigin Jaya Sravani Sirigineedi
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: