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

DA_France refresh #657

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

DA_France refresh #657

wants to merge 3 commits into from

Conversation

TinyRickC137
Copy link
Contributor

The entire DA_France machinery is moved to Archive (rational: *no clear and replicable definition for concept_naming, *DDL and source format mismatch [may be fixed based on findings from Documentation shared/(archived) LS analysis]).

The current approach (actual LS) is powered for only minor refreshes to provide efficient vocabulary maintenance.
Comments to define appropriate relationships for source fields and target table entities.
Updates statements are added in create_source_tables.sql.

…ar and replicable definition for concept_naming, *DDL and source format mismatch [may be fixed based on findings from Documentation shared/(archived) LS analysis]).

Current approach (actual LS) is powered for small refreshes only neither than to provide efficient  vocabulary  maintenance.
Comments to define appropriate relationships for source fields and target table entities.
Updates statements are added in create_source_tables.sql.
Copy link
Member

@Alexdavv Alexdavv left a comment

Choose a reason for hiding this comment

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

  1. Never ever recreate the files. Just move them in order to preserve the history. Once the old file is moved, you can create a new file with the same name in a separate commit.
  2. Updates are not allowed in the source tables. Any manipulations are to be done in the ls or manual work.
  3. Manual work is missing. How did we populate the manual tables?

@Alexdavv
Copy link
Member

Alexdavv commented Aug 3, 2022

@vladkorsik @TinyRickC137 I will run it now in devv5 schema because we get good results from the latest run. But let's fix it as requested within a new branch.

@TinyRickC137
Copy link
Contributor Author

@Alexdavv Agree with 1 and 2, but regarding 3:
During the LPD_Australia case you've asked me to drop all the manual work and insertions. So should we keep it or drop it?

@Alexdavv
Copy link
Member

Alexdavv commented Aug 4, 2022

We normally place it at the respective “manual_work” folder. What was wrong with the LPD_Australia so that I asked you to drop a manual part?

@TinyRickC137
Copy link
Contributor Author

It was altogether with manual checks in our private repositories and you said, that checks and insertions are for the person who does the vocab only, no need to make it available in the community nor to put it in our private repo.
So could you please create a guide on how to update manually vocabularies/all LPDs? That would be super helpful and make our approach more consistent.

@Alexdavv
Copy link
Member

Alexdavv commented Aug 4, 2022

Checks are different because we don’t want to replicate the standard code. For the rest - it comes to git.
A couple of good examples: LOINC, ICDs. And, hopefully, SNOMED from now.

@TinyRickC137
Copy link
Contributor Author

What's the priority for this task? Can you please provide good instruction on what to do with manual vocabularies? There are other LPDs coming sooner or later. The team would appreciate clear instructions to avoid misunderstanding.
And with this Pull request - if it wouldn't be merged, let's close it

@Alexdavv
Copy link
Member

@TinyRickC137 As I said, it was already run in devv5. So it will be merged once you fixed what was requested.

Restored source_tables
-- Manual work is populated
@vladkorsik
Copy link
Contributor

@Alexdavv please take a look at the Commit with rolback of Source.
And added the Mnaual tables updates

@TinyRickC137
Copy link
Contributor Author

More work has been done in branch 'da_france',
PR has been closed as outdated: #531

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