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

Write dozer.lock #1945

Merged
merged 1 commit into from
Aug 31, 2023
Merged

Write dozer.lock #1945

merged 1 commit into from
Aug 31, 2023

Conversation

Jesse-Bakker
Copy link
Contributor

Currently, this always writes the lock file to the current working directory.
Ideally we would follow the dozer-config.yaml around, but that is currently
impractical because of the implementation of the -c flag, combining *.sql and
dozer-config.*.

Copy link
Contributor

@chubei chubei left a comment

Choose a reason for hiding this comment

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

With this implementation, I think we can get rid of BuildPath.

It was designed to contain all information of a specific dozer execution, so user can restart dozer and continue from checkpoint.
Now that the contract file is moved out of it, there's no way to use data left in there anymore, so it makes more sense to just remove BuildPath and put everything that was previously in BuildPath to the HomeDir.

dozer-cli/src/simple/build/contract/mod.rs Outdated Show resolved Hide resolved
dozer-cli/src/simple/build/contract/mod.rs Outdated Show resolved Hide resolved
dozer-cli/src/simple/orchestrator.rs Outdated Show resolved Hide resolved
dozer-cli/src/simple/orchestrator.rs Outdated Show resolved Hide resolved
dozer-ingestion/tests/test_suite/connectors/dozer.rs Outdated Show resolved Hide resolved
dozer-tests/src/e2e_tests/runner/local.rs Outdated Show resolved Hide resolved
@Jesse-Bakker
Copy link
Contributor Author

If the contract is consistent with the endpoints, we can still re-use the build, the same way as when the contract was written to the build dir, I think.

Copy link
Contributor

@chubei chubei left a comment

Choose a reason for hiding this comment

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

Some minor comments

dozer-cli/src/simple/orchestrator.rs Outdated Show resolved Hide resolved
dozer-cli/src/simple/orchestrator.rs Outdated Show resolved Hide resolved
dozer-cli/src/simple/orchestrator.rs Outdated Show resolved Hide resolved
@chubei
Copy link
Contributor

chubei commented Aug 31, 2023

the same way as when the contract was written to the build dir

I didn't make myself clear.

Before this PR, it's possible to implement such a command:

dozer run app --build 1

where we compare the "current" contract with the contract stored in build v0001. If they are compatible, we start from checkpoint. If not, we report error.

Now that contract does not exist in the build directory anymore, such command is not possible. And this beats the purpose of the existence of build directories.

That's why I said we can get rid of BuildPath now. It's not useful anymore.

@Jesse-Bakker
Copy link
Contributor Author

Ah, as there is only ever one live build, the home dir in practice becomes the build directory. Luckily the contents of the home dir are an implementation detail, so I will defer this to after versioning

@Jesse-Bakker Jesse-Bakker added this pull request to the merge queue Aug 31, 2023
@chubei chubei removed this pull request from the merge queue due to a manual request Aug 31, 2023
@chubei chubei enabled auto-merge August 31, 2023 08:05
@Jesse-Bakker Jesse-Bakker linked an issue Aug 31, 2023 that may be closed by this pull request
@chubei chubei added this pull request to the merge queue Aug 31, 2023
Merged via the queue into getdozer:main with commit 18d6eab Aug 31, 2023
4 checks passed
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.

Output contract to dozer.lock file
2 participants