Skip to content
This repository has been archived by the owner on Oct 31, 2024. It is now read-only.

feat: add sync resource detector api and service and deployment detectors #129

Merged
merged 10 commits into from
Jun 16, 2021

Conversation

blumamir
Copy link
Contributor

@blumamir blumamir commented Jun 8, 2021

No description provided.

@blumamir blumamir requested a review from a team as a code owner June 8, 2021 14:44
Copy link
Collaborator

@rauno56 rauno56 left a comment

Choose a reason for hiding this comment

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

👍 Not much to say other than LGTM!

detectors/node/resource-detector-deployment/README.md Outdated Show resolved Hide resolved

loadJsonFile(path: string): any {
try {
return JSON.parse(fs.readFileSync(path).toString());
Copy link
Collaborator

Choose a reason for hiding this comment

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

return require(path); does the same thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know why, but it gives "Error: Cannot find module 'package.json'" when I try to require('package.json').
Probably the require base path is different somehow vs the fs.readFileSync alternative (which I believe can also bump into some edge cases where the process is not started from the service root)
If you have experience on how to write it so it works, that would be great :)

Copy link
Collaborator

@rauno56 rauno56 Jun 16, 2021

Choose a reason for hiding this comment

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

That's because it is looking for a package named package.json. ./package.json tells require to look for the file.

Both solutions are prone to errors from running the process from the outside of the project root folder.
I don't think there's a "canonical" way to get that without letting the user specify the root OR the path to the main package.json.

Best way I could come up with to more or less consistently find what you are looking for is this:

const path = require('path');

const findRootPackageJson = () => {
	const paths = require.main.paths.map((p) => {
		return path.resolve(p, '..', 'package.json');
	});
	for (const path of paths) {
		try {
			return {
				path,
				contents: require(path),
			};
		} catch (e) {}
	}

	return {};
}

console.log(require.main);
console.log(findRootPackageJson());

EDIT: Changed the catch syntax a tiny bit to support node 8.

detectors/node/resource-detector-service/README.md Outdated Show resolved Hide resolved
@blumamir blumamir merged commit 17d63f8 into master Jun 16, 2021
@blumamir blumamir deleted the service-detector branch June 16, 2021 08:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants